feat: branching #3246
feat: branching #3246
Conversation
WalkthroughAdds first-class branching: new Branch entity and DB schema, Key.branch relation and Project.branches, branch-aware queries/services/controllers and EE branch CRUD/copy/cleanup, DTOs/tests, frontend branch UI/hooks, export/translation filtering by branch. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as BranchController
participant Svc as BranchServiceImpl
participant Repo as BranchRepository
participant Copy as BranchCopyServiceSql
Client->>API: POST /v2/projects/{id}/branches (name, originBranchId)
API->>Svc: createBranch(projectId, name, originBranchId)
Svc->>Repo: validate origin & uniqueness
Repo-->>Svc: originBranch / null
Svc->>Repo: save(pending branch)
Svc->>Copy: copy(projectId, originBranch, newBranch)
Copy-->>Svc: done
Svc->>Repo: save(newBranch pending=false)
Svc-->>API: BranchModel
API-->>Client: 200
sequenceDiagram
autonumber
actor Client
participant API as TranslationsController
participant Svc as TranslationService
participant Repo as TranslationRepository
participant DB as DB
Client->>API: GET /translations?branch=feature
API->>Svc: getTranslations(..., branch="feature")
Svc->>Repo: getTranslations(langs, ns, branch, projectId, tags)
Repo->>DB: SELECT ... LEFT JOIN key.branch WHERE branch.name='feature' AND archived_at IS NULL
DB-->>Repo: rows
Repo-->>Svc: SimpleTranslationView[]
Svc-->>API: Map
API-->>Client: JSON
sequenceDiagram
autonumber
actor Client
participant API as BranchController
participant Svc as BranchServiceImpl
participant Pub as EventPublisher
participant Clean as BranchCleanupService
Client->>API: DELETE /v2/projects/{id}/branches/{branchId}
API->>Svc: deleteBranch(projectId, branchId)
Svc->>Svc: mark branch archived
Svc->>Pub: publish(OnBranchDeleted(branch))
Pub-->>Clean: OnBranchDeleted (AFTER_COMPLETION)
Clean->>...KeyRepository: fetch key ids by branch (paged)
loop per batch
Clean->>...KeyService: deleteMultiple(ids)
end
Clean->>...BranchRepository: delete branch
Estimated code review effort🎯 5 (Critical) | ⏱️ ~150 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dkrizan
force-pushed
the
danielkrizan/branching
branch
3 times, most recently
from
78c037f to
6371745
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 31
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
backend/data/src/main/kotlin/io/tolgee/service/queryBuilders/LanguageStatsProvider.kt (1)
80-101: Critical: Subquery lacks project correlation.Same issue as in
selectKeyCount- the subquery aggregates across all projects instead of correlating with the outer query's current project row.Apply this diff to correlate the subquery with the outer query's project:
private fun selectWordCount(state: TranslationState): Selection<Int> { val sub = query.subquery(Int::class.java) val project = sub.from(Project::class.java) val keyJoin = project.join(Project_.keys) joinDefaultBranch(sub, keyJoin) joinTargetTranslations(keyJoin, state) + sub.where( + cb.and( + sub.restriction, + cb.equal(project.get(Project_.id), this.project.get(Project_.id)) + ) + ) + val baseTranslationJoin = keyJoin.join(Key_.translations, JoinType.LEFT).also { translation -> translation.on( cb.equal(translation.get(Translation_.language), project.get(Project_.baseLanguage)), ) } val count = cb.sum(baseTranslationJoin.get(Translation_.wordCount)) val coalesceCount = cb.coalesce<Int>() coalesceCount.value(count) coalesceCount.value(0) return sub.select(coalesceCount) }Note: Use
this.projectto disambiguate from the localprojectvariable.backend/data/src/main/kotlin/io/tolgee/service/queryBuilders/ProjectStatsProvider.kt (1)
100-101: Bug: counting task IDs using Tag_.idCounting tasks should use Task_.id, not Tag_.id.
- return sub.select(cb.coalesce(cb.countDistinct(task.get(Tag_.id)), 0)) + return sub.select(cb.coalesce(cb.countDistinct(task.get(Task_.id)), 0))backend/data/src/main/kotlin/io/tolgee/dtos/IExportParams.kt (1)
109-125: filterBranch is not copied in copyPropsFrom()copyPropsFrom omits the new filterBranch, leading to lost branch filters when cloning params.
Apply:
fun copyPropsFrom(other: IExportParams) { this.languages = other.languages this.format = other.format this.structureDelimiter = other.structureDelimiter this.filterKeyId = other.filterKeyId this.filterKeyIdNot = other.filterKeyIdNot this.filterTag = other.filterTag this.filterTagIn = other.filterTagIn this.filterTagNotIn = other.filterTagNotIn this.filterKeyPrefix = other.filterKeyPrefix this.filterState = other.filterState this.filterNamespace = other.filterNamespace this.messageFormat = other.messageFormat this.supportArrays = other.supportArrays this.fileStructureTemplate = other.fileStructureTemplate this.escapeHtml = other.escapeHtml + this.filterBranch = other.filterBranch }
🧹 Nitpick comments (42)
webapp/src/eeSetup/eeModule.ee.tsx (1)
216-218: Consider guarding with PrivateRoute (align with auth gating)If other Project subroutes require auth, wrap BranchesView with PrivateRoute for consistency.
- <Route path={LINKS.PROJECT_BRANCHES.template}> - <BranchesView /> - </Route> + <PrivateRoute path={LINKS.PROJECT_BRANCHES.template}> + <BranchesView /> + </PrivateRoute>backend/data/src/main/kotlin/io/tolgee/service/branching/BranchService.kt (1)
10-10: Make originBranchId optional for flexibilityCreation may not always have/require an origin branch (e.g., empty branch, default base). Consider nullable with default.
- fun createBranch(projectId: Long, name: String, originBranchId: Long): Branch + fun createBranch(projectId: Long, name: String, originBranchId: Long? = null): Branchbackend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/dataImport/ImportTestData.kt (1)
350-356: Redundant author assignment in addCodeReferenceYou pass author to addCodeReference and also set author inside the lambda. Keep one; prefer the constructor arg for clarity. Same pattern appears further below (Lines 375–382, 389–395).
- addCodeReference(userAccount) { + addCodeReference(userAccount) { path = "./code/exist.extension" line = 10 fromImport = true - author = userAccount }webapp/src/component/branching/BranchLabel.tsx (1)
12-15: Type the ref target and add title for truncated content
- Add generics to forwardRef for correct ref typing.
- Provide title to show full branch name on hover.
-export const BranchLabel = React.forwardRef(function BranchLabel( - { value, placeholder, ...rest }: InputBaseComponentProps, - ref -) { +export const BranchLabel = React.forwardRef<HTMLDivElement, InputBaseComponentProps>(function BranchLabel( + { value, placeholder, ...rest }, + ref +) { return ( <StyledFakeInput tabIndex={0} ref={ref} - style={{ display: 'flex' }} + style={{ display: 'flex' }} {...(rest as any)} > <GitBranch02 height={20} width={20} style={{ marginRight: 10 }} /> - <div>{value || placeholder}</div> + <div title={String(value ?? placeholder ?? '')}>{value || placeholder}</div> </StyledFakeInput> ); });Also applies to: 17-25
backend/data/src/main/kotlin/io/tolgee/service/queryBuilders/ProjectStatsProvider.kt (1)
56-65: Scope default-branch filter to the same project (defensive)Current predicate counts keys where branch is null or branch.isDefault = true. Add a project match to avoid accidental cross-project linkage if data ever gets inconsistent.
sub.where( cb.and( key.get(Key_.project) equal project, cb.or( cb.isNull(key.get(Key_.branch)), cb.and( cb.isTrue(branch.get(Branch_.isDefault)), branch.get(Branch_.project) equal project ) ) ) )webapp/src/component/searchSelect/InfiniteSearchSelect.tsx (1)
50-55: Tighten types for inputComponent and menuAnchorOrigin
- inputComponent: allow intrinsic string types too by using React.ElementType.
- menuAnchorOrigin: use MUI’s PopoverOrigin (or MenuProps['anchorOrigin']) to stay aligned with upstream types.
- inputComponent?: React.ComponentType<InputBaseComponentProps>; - menuAnchorOrigin?: { - vertical: 'top' | 'bottom'; - horizontal: 'left' | 'right'; - }; + inputComponent?: React.ElementType<InputBaseComponentProps>; + menuAnchorOrigin?: import('@mui/material').PopoverOrigin;Usage remains the same:
- inputComponent: inputComponent || FakeInput, + inputComponent: inputComponent || FakeInput,Default stays:
anchorOrigin={menuAnchorOrigin || { vertical: 'top', horizontal: 'left' }}Also applies to: 75-77, 131-131, 166-171
backend/data/src/main/resources/db/changelog/schema.xml (1)
4671-4675: Remove transient UC creation/drop if possible (noise/fragility)You create UC_BRANCHORIGIN_BRANCH_ID_COL and later drop it, and also replace the (project_id, name) UC with a partial unique index. If this PR hasn’t shipped to any env, prefer squashing to the final state to avoid unnecessary create/drop churn.
If some environments already applied earlier changeSets, keep the drop to ensure forward-only migrations.
Also applies to: 4724-4729
backend/data/src/main/kotlin/io/tolgee/dtos/request/translation/TranslationFilters.kt (1)
161-164: Consider naming consistency.The field is correctly implemented with proper documentation. However, note that
ContentDeliveryConfigRequestusesfilterBranchwhile this class usesbranch. While both work correctly, consistent naming (e.g.,filterBranch) would align with other filter properties likefilterKeyId,filterTag, etc.backend/data/src/main/kotlin/io/tolgee/dtos/request/key/CreateKeyDto.kt (1)
51-51: Add @Schema annotation for API documentation.For consistency with other fields in this DTO and to provide clear API documentation, consider adding a
@Schemaannotation describing the branch parameter.Apply this diff:
+ @Schema(description = "The branch of the key. (When empty or null default branch will be used)") val branch: String? = null,webapp/src/component/branching/BranchSelector.tsx (1)
34-38: Consider simpler layout.The
Boxcomponent withdisplay="grid"seems unnecessary for a single child element. Unless this is preparing for future layout additions, a simpler approach would suffice.Apply this diff:
return ( - <Box display="grid"> + <Box> <BranchSelect branch={selected} onSelect={changeBranch} /> </Box> );Alternatively, you could remove the Box entirely:
return ( - <Box display="grid"> - <BranchSelect branch={selected} onSelect={changeBranch} /> - </Box> + <BranchSelect branch={selected} onSelect={changeBranch} /> );backend/data/src/main/kotlin/io/tolgee/dtos/request/key/ComplexEditKeyDto.kt (1)
63-64: LGTM! Consider adding validation for branch name format.The new optional
branchfield is well-documented and follows the pattern of other optional parameters in the DTO. However, consider whether branch names should have format validation (e.g., length constraints, allowed characters) to prevent invalid branch names from being accepted at the API layer.backend/data/src/main/kotlin/io/tolgee/dtos/request/translation/SetTranslationsWithKeyDto.kt (1)
38-39: LGTM! Consider consistent wording across DTOs.The new
branchfield is properly documented. For consistency, consider aligning the description with other DTOs (e.g., ComplexEditKeyDto mentions "If not provided, default branch will be used") to provide uniform API documentation across branch-aware endpoints.backend/data/src/main/kotlin/io/tolgee/service/queryBuilders/translationViewBuilder/QueryGlobalFiltering.kt (1)
267-284: Consider extracting branch filtering logic to reduce duplication.The branch filtering logic is nearly identical to the implementation in
ExportDataProvider(lines 157-173). Both handle null/default branch fallback and archived branch exclusion.Consider extracting this pattern into a shared utility method or extension function:
// Example utility fun CriteriaBuilder.branchFilter( branchJoin: Join<Key, Branch>, branchName: String? ): Predicate { return if (branchName.isNullOrEmpty()) { or( branchJoin.get(Branch_.id).isNull, isTrue(branchJoin.get(Branch_.isDefault)) ) } else { and( equal(branchJoin.get(Branch_.name), literal(branchName)), isNull(branchJoin.get(Branch_.archivedAt)) ) } }Then both locations could use:
whereConditions.add(cb.branchFilter(branchJoin, params.branch))This would make the logic more maintainable and reduce the risk of inconsistencies if the filtering rules change.
backend/data/src/main/kotlin/io/tolgee/service/bigMeta/BigMetaService.kt (1)
233-242: Add clarifying comment for complex branch filtering logic.The nested OR predicate at lines 233-242 handles branch matching for key similarity detection, but its purpose may not be immediately clear.
Consider adding a comment explaining the intent:
cb.or( // Match keys with specified branch (if not archived) cb.and( cb.equalNullable(branch.get(Branch_.name), key.branch), cb.isNull(branch.get(Branch_.archivedAt)), ), // Also match keys with no branch (legacy) or default branch cb.or( cb.isNull(root.get(Key_.branch)), cb.isTrue(branch.get(Branch_.isDefault)), ) )This clarifies that the logic includes keys that:
- Match the specified branch (when not archived), OR
- Have no branch association (backward compatibility), OR
- Are on the default branch
The complexity is justified here since BigMeta needs to find similar keys across branch contexts, but the comment would help future maintainers.
webapp/src/component/branching/BranchSelectField.tsx (1)
5-5: Improve type safety for Formik context.The
useFormikContexthook is typed asany, which eliminates type checking and could hide errors.Apply this diff to improve type safety:
- const { setFieldValue } = useFormikContext<any>(); + const { setFieldValue } = useFormikContext();Alternatively, if you need to constrain the form values, define a proper interface:
interface BranchFormValues { name: string; originBranchId: number; } const { setFieldValue } = useFormikContext<BranchFormValues>();backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/BranchTranslationsTestData.kt (1)
43-50: Consider using a builder pattern for tag creation.Tags are created via direct
Tag()instantiation, while other entities (labels, branches, languages) use the builder pattern. For consistency and to align with the test data builder conventions used elsewhere, consider adding tags through a builder method if one exists (e.g.,addTag { ... }).backend/app/src/test/kotlin/io/tolgee/service/TranslationServiceTest.kt (1)
21-26: Prefer setOf(...) over HashSet(listOf(...))Kotlin-idiomatic and clearer. No behavior change.
- languageTags = HashSet(listOf("en", "de")), + languageTags = setOf("en", "de"),Apply to all getTranslations calls above.
Also applies to: 39-44, 90-95, 107-112, 124-129
webapp/src/views/projects/translations/context/services/useEditService.tsx (1)
80-96: Only send branch when explicitly selected via routeUsing selected?.name may send a placeholder default object’s name; prefer the route-derived selectedName.
- branch: branchesService.selected?.name, + branch: branchesService.selectedName ?? undefined,webapp/src/ee/branching/components/BranchItem.tsx (1)
64-71: Improve a11y and test selector
- Add aria-label to delete button.
- Fix data-cy typo “branchs” → “branches”.
- <IconButton - data-cy="project-settings-branchs-remove-button" + <IconButton + aria-label="Delete branch" + data-cy="project-settings-branches-remove-button" size="small" onClick={() => onRemove(branch)} >webapp/src/ee/branching/components/BranchesList.tsx (2)
110-128: Fix test id and empty state text; guard protected deletion
- data-cy says labels; should be branches.
- Empty placeholder uses labels i18n key; switch to branches key.
- Prevent delete for protected branches too.
- <PaginatedHateoasList + <PaginatedHateoasList loadable={branchesLoadable} onPageChange={setPage} listComponent={TableGrid} - data-cy="project-settings-labels-list" + data-cy="project-settings-branches-list" emptyPlaceholder={ <Box m={2} display="flex" justifyContent="center"> <Typography color="textSecondary"> - {t('project_settings_no_labels_yet')} + {t('project_settings_no_branches_yet')} </Typography> </Box> } renderItem={(l: BranchModel) => ( <BranchItem branch={l} - onRemove={!l.isDefault ? deleteBranch : undefined} + onRemove={!l.isDefault && !l.isProtected ? deleteBranch : undefined} /> )} />Please confirm the i18n key exists or provide the correct one.
97-107: Wire edit permission when availableReplace hardcoded canEditBranches with permission check (as your TODO hints).
- const canEditBranches = true; // TODO satisfiesPermission('branches.edit') + const canEditBranches = satisfiesPermission?.('branches.edit') ?? true;webapp/src/component/branching/BranchSelect.tsx (1)
73-91: Optional: Wire search input to backend for large project branch lists.You pass
search={''}and never fetch more. If branch counts grow, allow live search and pagination.Minimal sketch:
+ const [search, setSearch] = useState(''); return ( <InfiniteSearchSelect items={branches} queryResult={loadable} itemKey={(item) => item.id} selected={selected} minHeight={false} - search={''} + search={search} + onSearchChange={setSearch} + onFetchMore={() => loadable.fetchNextPage?.()} renderItem={renderItem}Coordinate with API to thread a
searchquery to/branches.ee/backend/app/src/main/kotlin/io/tolgee/ee/service/branching/BranchCopyServiceSql.kt (2)
24-37: Validate branch-project consistency early.Fail fast if branches don’t belong to
projectId; otherwise copy becomes a silent no-op.Example:
@Transactional override fun copy(projectId: Long, sourceBranch: Branch, targetBranch: Branch) { require(sourceBranch.id != targetBranch.id) { "Source and target branch must differ" } + require(sourceBranch.project.id == projectId && targetBranch.project.id == projectId) { + "Branches must belong to the provided project" + }
52-55: Make namespace join sargable; avoidcoalesce()on indexed columns.
coalesce(tk.namespace_id,0) = coalesce(sk.namespace_id,0)inhibits index use. Prefer null-safe equality that’s index-friendly in Postgres.Postgres-specific alternative:
-join target_keys tk on tk.name = sk.name and coalesce(tk.namespace_id,0) = coalesce(sk.namespace_id,0) +join target_keys tk + on tk.name = sk.name + and ((tk.namespace_id = sk.namespace_id) or (tk.namespace_id is null and sk.namespace_id is null))Apply similarly in
copyKeys,copyTranslations,copyKeyMetas,copyKeyMeta*,copyTranslation*.Also applies to: 90-93, 136-140, 184-187, 231-234, 263-271, 338-341, 371-375
ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchController.kt (2)
31-36: Prefer centralized CORS config over controller-level wildcard
@CrossOrigin(origins = ["*"])weakens security guarantees. Move CORS to a global config (allowlisted origins per env) and drop the annotation here.
45-58: List permission can be read-onlyConsider relaxing GET to a read scope (e.g., KEYS_VIEW) to align with least privilege. POST/DELETE can keep KEYS_EDIT.
webapp/src/views/projects/translations/context/services/useBranchesService.tsx (2)
32-39: Infinite query not needed for a single, large pageYou request
size: 10000and don’t providegetNextPageParam. Use a simple query hook or setgetNextPageParamif multiple pages are expected.
18-24: Placeholder default branch with id 0 risks invalid writesIf UI submits before real data loads,
originBranchId(or consumers) may get 0. Prefer leaving branches empty until loaded or mark default only for display (never propagate its id).Consider:
- returning
branches: []untilisSuccess, and- keeping
defaultBranchseparate for UI hinting.webapp/src/ee/branching/components/BranchForm.tsx (1)
53-58: Ensure BranchSelect sets a real branch id before submitConsider disabling submit until
originBranchIdis truthy, or have BranchSelect emit only when real data is loaded. If needed, passdefaultBranchdown to avoid placeholder ids.ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchControllerTest.kt (1)
55-69: Also assert 201 Created on branch creationOnce the controller returns 201, assert status to lock API semantics.
- ).andAssertThatJson { + ).andIsCreated.andAssertThatJson {ee/backend/app/src/main/kotlin/io/tolgee/ee/service/branching/BranchServiceImpl.kt (4)
45-47: Redundant same‑project check
findByProjectIdAndId(projectId, originBranchId)already guarantees project ownership. The extra check is dead code.Apply:
- if (originBranch.project.id != projectId) { - throw BadRequestException(Message.ORIGIN_BRANCH_NOT_FOUND) - }
53-56: Ensure new branch ID is generated before copyIf
Branch.iduses IDENTITY/DB‑generated values,save()may not flush immediately. IfbranchCopyService.copy(...)relies onbranch.id, add a flush.branchRepository.save(branch) + entityManager.flush() // ensure branch.id is available for the copy branchCopyService.copy(projectId, originBranch, branch)
75-81: Publish deletion event after commitPublishing inside the open transaction risks listeners observing stale state. Prefer after‑commit publication or transactional event listeners.
Option A (publish after commit):
+import org.springframework.transaction.support.TransactionSynchronization +import org.springframework.transaction.support.TransactionSynchronizationManager ... - applicationContext.publishEvent(OnBranchDeleted(branch)) + TransactionSynchronizationManager.registerSynchronization(object : TransactionSynchronization { + override fun afterCommit() { + applicationContext.publishEvent(OnBranchDeleted(branch)) + } + })Option B: switch listeners to
@TransactionalEventListener(phase = AFTER_COMMIT)and keep current publish call.
63-73: Helper should be private; clarify name reuse vs. archived; add DB uniqueness
- Make this helper private to avoid unintended external use.
- The name collision check doesn’t filter archived branches. Decide if names can be reused after archival and adjust the repo method accordingly.
- Guarding by read‑then‑write is not atomic; add a DB unique constraint (e.g., partial unique index on (project_id, lower(name)) where archived_at is null) and map DataIntegrityViolation to BRANCH_ALREADY_EXISTS.
- fun createBranch(projectId: Long, name: String): Branch { + private fun createBranch(projectId: Long, name: String): Branch {ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchCopyIntegrationTest.kt (2)
78-84: Avoid hard timing assertions in CI
time.assert.isLessThan(3000)is environment‑dependent and flaky. Use a higher bound, conditional skip on CI, or assert functional readiness instead (e.g., wait forpending=false).
118-186: Tighten assertions and naming for readability
- Variable names
main*/branch*are swapped relative to actual objects; rename tocopy*/origin*.- Guard
.first()calls on comments/screenshots with presence checks to avoid brittle failures if data changes.backend/data/src/main/kotlin/io/tolgee/repository/KeyRepository.kt (2)
99-127: Search excludes non‑default branches
searchKeysfilters(br.id is null or br.is_default)and has no branch parameter. With branching, consider adding an optionalbrancharg to search within a branch (and default/null fallback).
60-71: Alias/style consistency (minor)You join
k.branch brbut selectk.branch.name. Preferbr.namefor clarity and to avoid accidental extra joins.- select new io.tolgee.dtos.queryResults.KeyView(k.id, k.name, ns.name, km.description, km.custom, k.branch.name) + select new io.tolgee.dtos.queryResults.KeyView(k.id, k.name, ns.name, km.description, km.custom, br.name)backend/data/src/main/kotlin/io/tolgee/service/key/KeyService.kt (2)
216-221: Branch resolution via collection scan
project.branches.find { ... }can trigger a lazy load and is O(n). Prefer repository lookup to avoid loading the whole collection.If feasible in this layer, use a
BranchRepository.findByProjectIdAndName(project.id, branch); otherwise, cache/default-branch accessor to avoid multiple loads.
200-202: Uniqueness check is non‑atomic
checkKeyNotExisting+ insert is prone to races. Ensure a DB unique constraint covers (project_id, namespace_id, branch_id, lower(name)) with appropriate null handling for default branch, and translate integrity violations toKEY_EXISTS.webapp/src/service/apiSchema.generated.ts (2)
1671-1677: Inconsistent param naming: filterBranch vs branch.Export endpoints use filterBranch; key/translation queries use branch. Consider aligning naming (e.g., filterBranch everywhere alongside other filter* params) while still in draft to avoid future breaking changes.
Also applies to: 1797-1803, 2558-2563, 12392-12397, 18307-18309, 13415-13416, 13851-13853, 17939-17941
413-419: Use descriptive operationIds for branch endpoints
Operations["all"], create_11, and delete_15 appear only inapiSchema.generated.ts; no client code references them. Rename these in the OpenAPI spec to listBranches, createBranch, deleteBranch, then regenerate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (103)
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/keys/KeyController.kt(2 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/translation/CreateOrUpdateTranslationsFacade.kt(2 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/translation/TranslationsController.kt(2 hunks)backend/api/src/main/kotlin/io/tolgee/component/KeyComplexEditHelper.kt(1 hunks)backend/api/src/main/kotlin/io/tolgee/controllers/ExportController.kt(1 hunks)backend/api/src/main/kotlin/io/tolgee/hateoas/contentDelivery/ContentDeliveryConfigModel.kt(1 hunks)backend/api/src/main/kotlin/io/tolgee/hateoas/key/KeyModel.kt(1 hunks)backend/api/src/main/kotlin/io/tolgee/hateoas/key/KeyModelAssembler.kt(1 hunks)backend/api/src/main/kotlin/io/tolgee/hateoas/key/KeyWithDataModel.kt(1 hunks)backend/api/src/main/kotlin/io/tolgee/hateoas/key/KeyWithDataModelAssembler.kt(1 hunks)backend/api/src/main/kotlin/io/tolgee/hateoas/key/KeyWithScreenshotsModelAssembler.kt(1 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/AllKeysControllerTest.kt(1 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translations/v2TranslationsController/TranslationsControllerModificationTest.kt(2 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translations/v2TranslationsController/TranslationsControllerViewTest.kt(1 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ImportController/SingleStepImportControllerTest.kt(1 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2KeyController/KeyControllerCreationTest.kt(2 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2KeyController/KeyControllerKeySearchTest.kt(1 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2KeyController/KeyControllerTest.kt(4 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ProjectsController/ProjectsControllerCreateTest.kt(2 hunks)backend/app/src/test/kotlin/io/tolgee/service/LanguageServiceTest.kt(1 hunks)backend/app/src/test/kotlin/io/tolgee/service/TranslationServiceTest.kt(3 hunks)backend/data/src/main/kotlin/io/tolgee/constants/Message.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/TestDataService.kt(5 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/BranchBuilder.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/ProjectBuilder.kt(3 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/BranchTestData.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/BranchTranslationsTestData.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/KeySearchTestData.kt(3 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/KeysTestData.kt(3 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/TranslationsTestData.kt(4 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/dataImport/ImportTestData.kt(3 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/ExportParamsDocs.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/IExportParams.kt(2 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/RelatedKeyDto.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/queryResults/KeyIdFindResult.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/queryResults/KeyView.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/request/ContentDeliveryConfigRequest.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/request/export/ExportParams.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/request/key/ComplexEditKeyDto.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/request/key/CreateKeyDto.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/request/key/EditKeyDto.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/request/translation/SetTranslationsWithKeyDto.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/request/translation/TranslationFilters.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/request/translation/importKeysResolvable/ImportKeysResolvableItemDto.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/events/OnBranchDeleted.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/model/Project.kt(4 hunks)backend/data/src/main/kotlin/io/tolgee/model/branching/Branch.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/model/contentDelivery/ContentDeliveryConfig.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/model/key/Key.kt(4 hunks)backend/data/src/main/kotlin/io/tolgee/model/translation/Translation.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/repository/KeyMetaRepository.kt(2 hunks)backend/data/src/main/kotlin/io/tolgee/repository/KeyRepository.kt(9 hunks)backend/data/src/main/kotlin/io/tolgee/repository/TranslationRepository.kt(2 hunks)backend/data/src/main/kotlin/io/tolgee/service/bigMeta/BigMetaService.kt(3 hunks)backend/data/src/main/kotlin/io/tolgee/service/bigMeta/KeysDistanceUtil.kt(4 hunks)backend/data/src/main/kotlin/io/tolgee/service/branching/BranchCopyService.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/service/branching/BranchService.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/service/branching/BranchServiceOssStub.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/service/export/dataProvider/ExportDataProvider.kt(5 hunks)backend/data/src/main/kotlin/io/tolgee/service/key/KeyService.kt(6 hunks)backend/data/src/main/kotlin/io/tolgee/service/key/ResolvingKeyImporter.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/service/project/ProjectCreationService.kt(2 hunks)backend/data/src/main/kotlin/io/tolgee/service/queryBuilders/LanguageStatsProvider.kt(4 hunks)backend/data/src/main/kotlin/io/tolgee/service/queryBuilders/ProjectStatsProvider.kt(2 hunks)backend/data/src/main/kotlin/io/tolgee/service/queryBuilders/translationViewBuilder/QueryGlobalFiltering.kt(3 hunks)backend/data/src/main/kotlin/io/tolgee/service/translation/TranslationService.kt(2 hunks)backend/data/src/main/resources/db/changelog/schema.xml(1 hunks)backend/data/src/test/kotlin/io/tolgee/unit/bigMeta/KeysDistanceUnitTestData.kt(1 hunks)e2e/cypress/support/dataCyType.d.ts(2 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchController.kt(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/hateoas/assemblers/BranchModelAssembler.kt(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/hateoas/model/branching/BranchModel.kt(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/hateoas/model/branching/CreateBranchModel.kt(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/repository/BranchRepository.kt(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/service/branching/BranchCleanupService.kt(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/service/branching/BranchCopyServiceSql.kt(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/service/branching/BranchServiceImpl.kt(1 hunks)ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchControllerTest.kt(1 hunks)ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchCopyIntegrationTest.kt(1 hunks)webapp/src/component/branching/BranchLabel.tsx(1 hunks)webapp/src/component/branching/BranchSelect.tsx(1 hunks)webapp/src/component/branching/BranchSelectField.tsx(1 hunks)webapp/src/component/branching/BranchSelector.tsx(1 hunks)webapp/src/component/branching/DefaultBranchChip.tsx(1 hunks)webapp/src/component/common/ClipboardCopy.tsx(1 hunks)webapp/src/component/searchSelect/InfiniteSearchSelect.tsx(6 hunks)webapp/src/constants/GlobalValidationSchema.tsx(1 hunks)webapp/src/constants/links.tsx(3 hunks)webapp/src/ee/branching/BranchesView.tsx(1 hunks)webapp/src/ee/branching/components/BranchForm.tsx(1 hunks)webapp/src/ee/branching/components/BranchItem.tsx(1 hunks)webapp/src/ee/branching/components/BranchModal.tsx(1 hunks)webapp/src/ee/branching/components/BranchNameChip.tsx(1 hunks)webapp/src/ee/branching/components/BranchesList.tsx(1 hunks)webapp/src/eeSetup/eeModule.ee.tsx(4 hunks)webapp/src/service/apiSchema.generated.ts(36 hunks)webapp/src/views/projects/ProjectRouter.tsx(1 hunks)webapp/src/views/projects/translations/KeyCreateForm/KeyCreateForm.tsx(3 hunks)webapp/src/views/projects/translations/TranslationHeader/TranslationControls.tsx(2 hunks)webapp/src/views/projects/translations/context/TranslationsContext.ts(3 hunks)webapp/src/views/projects/translations/context/services/useBranchesService.tsx(1 hunks)webapp/src/views/projects/translations/context/services/useEditService.tsx(3 hunks)webapp/src/views/projects/translations/context/services/useTranslationsService.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (28)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/BranchTranslationsTestData.kt (5)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/TestDataBuilder.kt (2)
addUserAccount(36-51)addProject(53-61)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/ProjectBuilder.kt (4)
addPermission(74-74)addEnglish(148-153)addGerman(155-161)addBranch(90-90)backend/data/src/main/kotlin/io/tolgee/model/translation/Translation.kt (1)
addLabel(131-134)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/KeyBuilder.kt (1)
addMeta(44-47)backend/data/src/main/kotlin/io/tolgee/model/key/KeyMeta.kt (2)
addComment(56-64)addCodeReference(66-74)
webapp/src/views/projects/translations/TranslationHeader/TranslationControls.tsx (1)
webapp/src/component/branching/BranchSelector.tsx (1)
BranchSelector(12-39)
webapp/src/component/branching/BranchSelectField.tsx (1)
webapp/src/component/branching/BranchSelect.tsx (1)
BranchSelect(25-92)
webapp/src/ee/branching/BranchesView.tsx (3)
webapp/src/views/projects/BaseProjectView.tsx (1)
BaseProjectView(19-60)webapp/src/constants/links.tsx (1)
LINKS(69-441)webapp/src/ee/branching/components/BranchesList.tsx (1)
BranchesList(21-135)
webapp/src/views/projects/translations/context/services/useEditService.tsx (1)
webapp/src/views/projects/translations/context/services/useBranchesService.tsx (1)
useBranchesService(26-72)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/BranchTestData.kt (1)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/ProjectBuilder.kt (1)
addBranch(90-90)
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translations/v2TranslationsController/TranslationsControllerModificationTest.kt (1)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/TestDataService.kt (2)
saveTestData(77-83)saveTestData(85-121)
webapp/src/eeSetup/eeModule.ee.tsx (2)
webapp/src/constants/links.tsx (1)
LINKS(69-441)webapp/src/ee/branching/BranchesView.tsx (1)
BranchesView(7-27)
webapp/src/ee/branching/components/BranchModal.tsx (2)
webapp/src/service/apiSchema.generated.ts (1)
components(1072-6105)webapp/src/ee/branching/components/BranchForm.tsx (2)
BranchFormValues(13-16)BranchForm(18-63)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/dataImport/ImportTestData.kt (1)
backend/data/src/main/kotlin/io/tolgee/model/key/KeyMeta.kt (1)
addCodeReference(66-74)
webapp/src/views/projects/translations/context/services/useBranchesService.tsx (1)
webapp/src/views/projects/translations/context/services/useTranslationsService.tsx (1)
useTranslationsService(74-435)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/KeySearchTestData.kt (1)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/ProjectBuilder.kt (1)
addBranch(90-90)
webapp/src/ee/branching/components/BranchItem.tsx (4)
webapp/src/service/apiSchema.generated.ts (1)
components(1072-6105)webapp/src/ee/branching/components/BranchNameChip.tsx (1)
BranchNameChip(13-15)webapp/src/component/common/ClipboardCopy.tsx (1)
ClipboardCopy(13-53)webapp/src/component/branching/DefaultBranchChip.tsx (1)
DefaultBranchChip(5-8)
webapp/src/views/projects/translations/context/TranslationsContext.ts (2)
webapp/src/views/projects/translations/context/services/useBranchesService.tsx (1)
useBranchesService(26-72)webapp/src/views/projects/translations/context/services/useEditService.tsx (1)
useEditService(36-251)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/KeysTestData.kt (2)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/ProjectBuilder.kt (1)
addBranch(90-90)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/KeyBuilder.kt (1)
setDescription(100-103)
backend/data/src/main/kotlin/io/tolgee/service/export/dataProvider/ExportDataProvider.kt (1)
backend/data/src/main/kotlin/io/tolgee/service/queryBuilders/translationViewBuilder/QueryGlobalFiltering.kt (1)
filterBranch(267-284)
webapp/src/views/projects/ProjectRouter.tsx (2)
webapp/src/constants/links.tsx (1)
LINKS(69-441)webapp/src/views/projects/translations/TranslationsView.tsx (1)
TranslationsView(9-30)
backend/data/src/main/kotlin/io/tolgee/service/queryBuilders/translationViewBuilder/QueryGlobalFiltering.kt (1)
backend/data/src/main/kotlin/io/tolgee/service/export/dataProvider/ExportDataProvider.kt (1)
filterBranch(158-174)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/TranslationsTestData.kt (3)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/ProjectBuilder.kt (6)
addBranch(90-90)addKey(82-82)addKey(105-111)addKey(113-115)addKey(117-128)addTranslation(130-130)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/KeyBuilder.kt (1)
setDescription(100-103)backend/data/src/main/kotlin/io/tolgee/model/key/KeyMeta.kt (1)
addComment(56-64)
webapp/src/component/branching/BranchSelector.tsx (3)
webapp/src/views/projects/translations/context/services/useBranchesService.tsx (1)
useBranchesService(26-72)webapp/src/constants/links.tsx (1)
LINKS(69-441)webapp/src/component/branching/BranchSelect.tsx (1)
BranchSelect(25-92)
webapp/src/views/projects/translations/context/services/useTranslationsService.tsx (1)
webapp/src/views/projects/translations/context/services/useBranchesService.tsx (1)
useBranchesService(26-72)
webapp/src/component/branching/BranchSelect.tsx (5)
webapp/src/views/projects/translations/context/services/useBranchesService.tsx (1)
useBranchesService(26-72)webapp/src/component/searchSelect/SearchSelect.tsx (1)
SelectItem(8-11)webapp/src/component/branching/DefaultBranchChip.tsx (1)
DefaultBranchChip(5-8)webapp/src/component/searchSelect/InfiniteSearchSelect.tsx (1)
InfiniteSearchSelect(58-209)webapp/src/component/branching/BranchLabel.tsx (1)
BranchLabel(12-27)
webapp/src/ee/branching/components/BranchForm.tsx (5)
webapp/src/service/apiSchema.generated.ts (1)
components(1072-6105)webapp/src/component/common/form/StandardForm.tsx (1)
StandardForm(37-121)webapp/src/constants/GlobalValidationSchema.tsx (1)
Validation(44-536)webapp/src/component/FormField.tsx (1)
FieldLabel(20-30)webapp/src/component/branching/BranchSelectField.tsx (1)
BranchSelectField(4-15)
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/translation/CreateOrUpdateTranslationsFacade.kt (1)
backend/data/src/main/kotlin/io/tolgee/service/key/KeyService.kt (2)
create(123-157)create(192-202)
backend/data/src/main/kotlin/io/tolgee/service/key/KeyService.kt (2)
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/translation/CreateOrUpdateTranslationsFacade.kt (1)
create(39-54)backend/data/src/main/kotlin/io/tolgee/model/key/Key.kt (1)
Key(41-131)
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translations/v2TranslationsController/TranslationsControllerViewTest.kt (1)
backend/testing/src/main/kotlin/io/tolgee/fixtures/ProjectApiKeyAuthRequestPerformer.kt (1)
performProjectAuthGet(47-49)
ee/backend/app/src/main/kotlin/io/tolgee/ee/service/branching/BranchServiceImpl.kt (2)
backend/data/src/main/kotlin/io/tolgee/service/branching/BranchService.kt (2)
createBranch(10-10)getBranch(9-9)backend/data/src/main/kotlin/io/tolgee/service/branching/BranchServiceOssStub.kt (2)
createBranch(16-18)getBranch(13-15)
webapp/src/ee/branching/components/BranchesList.tsx (6)
webapp/src/service/apiSchema.generated.ts (1)
components(1072-6105)webapp/src/service/http/useQueryApi.ts (2)
useApiMutation(224-265)useApiQuery(119-141)webapp/src/ee/branching/components/BranchForm.tsx (1)
BranchFormValues(13-16)webapp/src/hooks/confirmation.tsx (1)
confirmation(5-7)webapp/src/ee/branching/components/BranchItem.tsx (1)
BranchItem(47-76)webapp/src/ee/branching/components/BranchModal.tsx (1)
BranchModal(12-37)
🪛 Biome (2.1.2)
webapp/src/views/projects/translations/context/services/useBranchesService.tsx
[error] 30-30: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build backend 🏗️
| @ProjectJWTAuthTestMethod | ||
| @Test | ||
| fun `creates key with default branch (branch parameter not provided)`() { | ||
| performProjectAuthPost("keys", CreateKeyDto(name = "super_key_to_main_branch", branch = "dev")) | ||
| .andIsCreated.andAssertThatJson { | ||
| node("id").isValidId | ||
| node("name").isEqualTo("super_key_to_main_branch") | ||
| node("branch").isEqualTo("dev") | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix misleading test name.
The test name states "creates key with default branch (branch parameter not provided)" but the test actually provides an explicit branch parameter (branch = "dev"). This is misleading and contradicts the test's actual behavior.
Apply this diff to correct the test name:
- fun `creates key with default branch (branch parameter not provided)`() {
- performProjectAuthPost("keys", CreateKeyDto(name = "super_key_to_main_branch", branch = "dev"))
+ fun `creates key with explicit branch`() {
+ performProjectAuthPost("keys", CreateKeyDto(name = "super_key_to_dev_branch", branch = "dev"))
.andIsCreated.andAssertThatJson {
node("id").isValidId
- node("name").isEqualTo("super_key_to_main_branch")
+ node("name").isEqualTo("super_key_to_dev_branch")
node("branch").isEqualTo("dev")
}
}🤖 Prompt for AI Agents
In
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2KeyController/KeyControllerCreationTest.kt
around lines 86 to 95, the test name claims it verifies creation with the
default branch when branch is not provided, but the test call supplies branch =
"dev"; update the test name to reflect that an explicit branch is provided
(e.g., rename the function to indicate "creates key with provided branch (branch
parameter provided)") so the name matches the test behavior.
| @Test | ||
| @ProjectJWTAuthTestMethod | ||
| fun `it search in default branch only`() { | ||
| saveAndPrepare() | ||
| performProjectAuthGet("keys/search?search=this-is-branched-key&languageTag=de").andAssertThatJson { | ||
| node("_embedded").isAbsent() | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Fix test name consistency and verify test data setup.
-
Test name inconsistency: The test is named "it search in default branch only" but should be "it searches in default branch only" to match the naming pattern of other tests in this file (e.g., "it searches", "it searches for prefix").
-
Verify test data: Ensure that
KeySearchTestDatacreates a key named "this-is-branched-key" in a non-default branch. Without seeing the test data setup, it's unclear if this branched key actually exists, which could lead to a false positive test.
Apply this diff to fix the test name:
- fun `it search in default branch only`() {
+ fun `it searches in default branch only`() {Run the following script to verify the test data includes a branched key:
🏁 Script executed:
#!/bin/bash
# Verify that KeySearchTestData creates a branched key named "this-is-branched-key"
ast-grep --pattern $'class KeySearchTestData {
$$$
}'
rg -A 20 "class KeySearchTestData" --type=ktLength of output: 26
#!/bin/bash
# Locate KeySearchTestData definition
find . -type f -name "*KeySearchTestData*.kt"
# Inspect the class declaration for test data setup
rg -n "class KeySearchTestData" -C5 .
# Search for the branched key name within the repository
rg -n "this-is-branched-key" -C5 .🏁 Script executed:
#!/bin/bash
# Locate the KeySearchTestData Kotlin file
find . -type f -name "*KeySearchTestData*.kt"
# Search for the branched key literal across all Kotlin files
rg -n "this-is-branched-key" -g '*.kt'Length of output: 490
🏁 Script executed:
#!/bin/bash
# Show class header to see project and branch setup
sed -n '1,50p' backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/KeySearchTestData.kt
# Inspect addKeyWithTranslations implementation
rg -n "fun addKeyWithTranslations" -C10 backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/KeySearchTestData.kt
# Search for branch creation within the file
rg -n "branch" -C5 backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/KeySearchTestData.ktLength of output: 2500
Rename test function for naming consistency
Apply the diff:
- fun `it search in default branch only`() {
+ fun `it searches in default branch only`() {🤖 Prompt for AI Agents
In
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2KeyController/KeyControllerKeySearchTest.kt
around lines 148 to 155, rename the test function from `it search in default
branch only` to `it searches in default branch only` to match project naming
conventions; update the function declaration accordingly (keep annotations and
body unchanged) and ensure any references or usages are adjusted to the new
name.
| private fun saveAllKeyDependants(keyBuilders: List<KeyBuilder>) { | ||
| val metas = keyBuilders.map { it.data.meta?.self }.filterNotNull() | ||
| tagService.saveAll(metas.flatMap { it.tags }) | ||
| keyCodeReferenceRepository.saveAll(metas.flatMap { it.codeReferences }) | ||
| keyMetaService.saveAll(metas) | ||
| } |
There was a problem hiding this comment.
Potential FK/cascade issue: saving code references before metas
You persist codeReferences before KeyMeta, which can break referential integrity or double-persist with cascades. Save metas first, then code references (or rely solely on cascade). Also, align with import path which uses keyMetaService.saveAllCodeReferences.
private fun saveAllKeyDependants(keyBuilders: List<KeyBuilder>) {
val metas = keyBuilders.map { it.data.meta?.self }.filterNotNull()
tagService.saveAll(metas.flatMap { it.tags })
- keyCodeReferenceRepository.saveAll(metas.flatMap { it.codeReferences })
- keyMetaService.saveAll(metas)
+ // persist metas first to ensure FK targets exist
+ keyMetaService.saveAll(metas)
+ // unify with import path saving approach
+ keyMetaService.saveAllCodeReferences(metas.flatMap { it.codeReferences })
}If you adopt this, the KeyCodeReferenceRepository dependency becomes unused; remove it to avoid dead deps.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private fun saveAllKeyDependants(keyBuilders: List<KeyBuilder>) { | |
| val metas = keyBuilders.map { it.data.meta?.self }.filterNotNull() | |
| tagService.saveAll(metas.flatMap { it.tags }) | |
| keyCodeReferenceRepository.saveAll(metas.flatMap { it.codeReferences }) | |
| keyMetaService.saveAll(metas) | |
| } | |
| private fun saveAllKeyDependants(keyBuilders: List<KeyBuilder>) { | |
| val metas = keyBuilders.map { it.data.meta?.self }.filterNotNull() | |
| tagService.saveAll(metas.flatMap { it.tags }) | |
| // persist metas first to ensure FK targets exist | |
| keyMetaService.saveAll(metas) | |
| // unify with import path saving approach | |
| keyMetaService.saveAllCodeReferences(metas.flatMap { it.codeReferences }) | |
| } |
🤖 Prompt for AI Agents
In
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/TestDataService.kt
around lines 465 to 470, the current save order persists codeReferences before
KeyMeta which can break FK integrity or cause double-persist with cascades;
change the order to persist metas first via keyMetaService.saveAll(metas) and
then save code references using
keyMetaService.saveAllCodeReferences(metas.flatMap { it.codeReferences }) (or
rely solely on cascade and omit explicit codeReferences save), and after
switching to keyMetaService for code references remove the now-unused
keyCodeReferenceRepository dependency/import to avoid dead code.
| @ActivityLoggedProp | ||
| @Column(length = 200) | ||
| var name: String = "", | ||
|
|
||
| @ActivityLoggedProp | ||
| var isDefault: Boolean = false, | ||
|
|
||
| @ActivityLoggedProp | ||
| var isProtected: Boolean = false, | ||
|
|
||
| @ActivityLoggedProp | ||
| var archivedAt: Date? = null, | ||
|
|
||
| @Column(name = "pending") | ||
| var pending: Boolean = false, | ||
|
|
There was a problem hiding this comment.
Nullability mismatch for name and pending can cause NPEs
- Kotlin fields are non-null (name: String, pending: Boolean), but DB columns are nullable in schema.xml. Hydration can produce nulls and NPEs.
Code-side:
- @Column(length = 200)
+ @Column(length = 200, nullable = false)
var name: String = "",
@@
- @Column(name = "pending")
- var pending: Boolean = false,
+ @Column(name = "pending", nullable = false)
+ var pending: Boolean = false,Schema-side (see schema.xml comments below) ensure:
- branch.name NOT NULL
- branch.pending NOT NULL DEFAULT false
| @OneToMany(fetch = FetchType.LAZY, cascade = [CascadeType.PERSIST], mappedBy = "project") | ||
| @SQLRestriction("archived_at IS NULL") | ||
| var branches: MutableList<Branch> = mutableListOf() |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the SQLRestriction column name.
The @SQLRestriction annotation uses "archived_at IS NULL". Ensure this column name matches the actual database column name in the Branch entity. Hibernate typically uses snake_case for column names by default, but it's worth confirming that the Branch.archivedAt field maps to archived_at in the database schema.
Run the following script to verify the column mapping:
🏁 Script executed:
#!/bin/bash
# Description: Check Branch entity's archivedAt column mapping
# Search for the archivedAt field definition in Branch entity
ast-grep --pattern $'class Branch {
$$$
archivedAt$$$
$$$
}'
# Also search for any @Column annotations on archivedAt
rg -n '@Column.*archivedAt|archivedAt.*@Column' --type kotlin -A 2 -B 2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Locate the Kotlin file defining Branch
branch_file=$(rg -l "class Branch" -g "*.kt")
echo "Branch file: $branch_file"
# Search for archivedAt definition and annotations
rg -n "archivedAt" -C3 "$branch_file"Length of output: 2384
SQLRestriction references nonexistent column archived_at in Branch
The filter @SQLRestriction("archived_at IS NULL") on branches points to a column not mapped in Branch.kt (no archivedAt property or @Column(name = "archived_at")). Add the field/mapping or remove/update the restriction.
🤖 Prompt for AI Agents
In backend/data/src/main/kotlin/io/tolgee/model/Project.kt around lines 148-150,
the @SQLRestriction("archived_at IS NULL") on the branches collection refers to
a non-existent column in Branch; fix by either adding a nullable mapped property
in Branch (e.g., an Instant/LocalDateTime or appropriate date-time type with
@Column(name = "archived_at") and nullable) so the column exists for the
restriction, or remove/update the @SQLRestriction to reference an existing
mapped column; ensure the chosen fix is applied consistently (add the field to
Branch.kt with proper import/annotations and database migration, or
delete/change the restriction in Project.kt).
| const match = useRouteMatch(); | ||
| const routeBranch = match.params[PARAMS.TRANSLATIONS_BRANCH]; | ||
|
|
||
| projectId = projectId || useProject().id; |
There was a problem hiding this comment.
Hooks rule violation: calling a hook conditionally
useProject() is invoked conditionally via projectId || useProject().id, breaking hooks order. Call it unconditionally and derive an id.
Apply:
-import { useProject } from 'tg.hooks/useProject';
+import { useProject } from 'tg.hooks/useProject';
@@
-export const useBranchesService = ({ projectId, enabled = true }: Props) => {
+export const useBranchesService = ({ projectId, enabled = true }: Props) => {
const match = useRouteMatch();
const routeBranch = match.params[PARAMS.TRANSLATIONS_BRANCH];
-
- projectId = projectId || useProject().id;
+ const projectCtx = useProject();
+ const resolvedProjectId = projectId ?? projectCtx.id;
@@
- path: { projectId: projectId },
+ path: { projectId: resolvedProjectId },🧰 Tools
🪛 Biome (2.1.2)
[error] 30-30: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🤖 Prompt for AI Agents
In
webapp/src/views/projects/translations/context/services/useBranchesService.tsx
around line 30, the code conditionally calls the hook useProject() via projectId
|| useProject().id which violates React hooks rules; always call useProject()
unconditionally and then derive the id by using const project = useProject();
projectId = projectId || project.id (or better, compute a local const
resolvedProjectId = projectId ?? project.id and use that) so hooks order stays
stable.
| const loadableBranches = useApiInfiniteQuery({ | ||
| url: '/v2/projects/{projectId}/branches', | ||
| method: 'get', | ||
| path: { projectId: projectId }, | ||
| query: { | ||
| size: 10000, | ||
| }, | ||
| options: { | ||
| keepPreviousData: true, | ||
| refetchOnMount: true, | ||
| noGlobalLoading: true, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Pass through the enabled flag to the query
You accept enabled but don’t use it. Wire it to query options.
const loadableBranches = useApiInfiniteQuery({
@@
- options: {
+ options: {
keepPreviousData: true,
refetchOnMount: true,
noGlobalLoading: true,
+ enabled,
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const loadableBranches = useApiInfiniteQuery({ | |
| url: '/v2/projects/{projectId}/branches', | |
| method: 'get', | |
| path: { projectId: projectId }, | |
| query: { | |
| size: 10000, | |
| }, | |
| options: { | |
| keepPreviousData: true, | |
| refetchOnMount: true, | |
| noGlobalLoading: true, | |
| }, | |
| }); | |
| const loadableBranches = useApiInfiniteQuery({ | |
| url: '/v2/projects/{projectId}/branches', | |
| method: 'get', | |
| path: { projectId: projectId }, | |
| query: { | |
| size: 10000, | |
| }, | |
| options: { | |
| keepPreviousData: true, | |
| refetchOnMount: true, | |
| noGlobalLoading: true, | |
| enabled, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In
webapp/src/views/projects/translations/context/services/useBranchesService.tsx
around lines 32 to 44, the hook accepts an enabled parameter but doesn't pass it
to the query options; update the useApiInfiniteQuery call to forward the enabled
flag by adding options.enabled: enabled (or options.enabled: Boolean(enabled) if
needed) so the query respects the caller's enabled state while preserving the
existing options (keepPreviousData, refetchOnMount, noGlobalLoading).
| const loadedBranches = loadableBranches.data?.pages.flatMap( | ||
| (p) => p._embedded?.branches ?? [] | ||
| ); | ||
|
|
||
| const data = | ||
| loadableBranches.isFetched && loadedBranches!.length > 0 | ||
| ? loadableBranches.data!.pages.flatMap((p) => p._embedded?.branches ?? []) | ||
| : Array.from([defaultBranchObject]); | ||
|
|
There was a problem hiding this comment.
Avoid unsafe non-null assertion and crash when request errors
isFetched && loadedBranches!.length > 0 can throw when data is undefined (errors still set isFetched=true). Use optional chaining and/or isSuccess.
- const loadedBranches = loadableBranches.data?.pages.flatMap(
- (p) => p._embedded?.branches ?? []
- );
+ const loadedBranches =
+ loadableBranches.data?.pages.flatMap((p) => p._embedded?.branches ?? []) ??
+ [];
@@
- const data =
- loadableBranches.isFetched && loadedBranches!.length > 0
- ? loadableBranches.data!.pages.flatMap((p) => p._embedded?.branches ?? [])
- : Array.from([defaultBranchObject]);
+ const data =
+ (loadableBranches.isSuccess && loadedBranches.length > 0 && loadedBranches) ||
+ [defaultBranchObject];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const loadedBranches = loadableBranches.data?.pages.flatMap( | |
| (p) => p._embedded?.branches ?? [] | |
| ); | |
| const data = | |
| loadableBranches.isFetched && loadedBranches!.length > 0 | |
| ? loadableBranches.data!.pages.flatMap((p) => p._embedded?.branches ?? []) | |
| : Array.from([defaultBranchObject]); | |
| const loadedBranches = | |
| loadableBranches.data?.pages.flatMap((p) => p._embedded?.branches ?? []) ?? []; | |
| const data = | |
| (loadableBranches.isSuccess && loadedBranches.length > 0 && loadedBranches) || | |
| [defaultBranchObject]; |
🤖 Prompt for AI Agents
In
webapp/src/views/projects/translations/context/services/useBranchesService.tsx
around lines 46 to 54, the condition uses a non-null assertion (loadedBranches!)
and can throw when data is undefined even though isFetched is true; replace that
check with a safe one (e.g. loadableBranches.isSuccess &&
(loadedBranches?.length ?? 0) > 0) or explicitly check loadedBranches?.length >
0, and reuse the already computed loadedBranches variable for the true branch
instead of recomputing flatMap; ensure you never use "!" on loadedBranches and
always guard access with optional chaining or isSuccess so the fallback
defaultBranchObject is used when data is missing or the request errored.
| const urlBranch = routeBranch | ||
| ? data?.find((b) => b.name === routeBranch) | ||
| : undefined; | ||
|
|
||
| const defaultBranch = loadableBranches.isFetched | ||
| ? loadedBranches?.find((b) => b.isDefault) || defaultBranchObject | ||
| : null; | ||
|
|
||
| const selected = routeBranch ? (urlBranch ? urlBranch : null) : defaultBranch; | ||
|
|
||
| return { | ||
| branches: data, | ||
| selected: selected, | ||
| default: defaultBranch, | ||
| selectedName: routeBranch, | ||
| loadable: loadableBranches, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
selectedName can become invalid; return the actual selected name
If URL branch is unknown, selected becomes null but selectedName stays as the invalid URL value, causing queries to filter by a non-existent branch and return no data.
Also, prefer useParams over useRouteMatch for params access.
-import { useRouteMatch } from 'react-router-dom';
+import { useParams } from 'react-router-dom';
@@
- const match = useRouteMatch();
- const routeBranch = match.params[PARAMS.TRANSLATIONS_BRANCH];
+ const params = useParams<Record<string, string | undefined>>();
+ const routeBranch = params[PARAMS.TRANSLATIONS_BRANCH];
@@
- const selected = routeBranch ? (urlBranch ? urlBranch : null) : defaultBranch;
+ const selected = routeBranch ? (urlBranch ?? null) : defaultBranch;
@@
- selectedName: routeBranch,
+ selectedName: selected?.name,To ensure no callers rely on the previous selectedName behavior or on optional projectId, list usages:
🏁 Script executed:
#!/bin/bash
rg -nP --type=ts --type=tsx -C2 '\buseBranchesService\s*\('Length of output: 27
🏁 Script executed:
#!/bin/bash
rg -n '\buseBranchesService\s*\(' -C2Length of output: 1795
🏁 Script executed:
#!/bin/bash
echo "---- useBranchesService.tsx (lines 1-80) ----"
sed -n '1,80p' webapp/src/views/projects/translations/context/services/useBranchesService.tsx
echo "---- useTranslationsService.tsx (lines 70-110) ----"
sed -n '70,110p' webapp/src/views/projects/translations/context/services/useTranslationsService.tsxLength of output: 3357
🏁 Script executed:
#!/bin/bash
sed -n '1,200p' webapp/src/views/projects/translations/context/services/useTranslationsService.tsxLength of output: 6125
Update useBranchesService to return the actual branch name and use useParams
- Replace useRouteMatch with useParams to read URL params
- Derive selectedName from selected?.name so it becomes undefined when the branch is invalid
- (Optional) simplify the selected expression using nullish coalescing
-import { useRouteMatch } from 'react-router-dom';
+import { useParams } from 'react-router-dom';
@@
- const match = useRouteMatch();
- const routeBranch = match.params[PARAMS.TRANSLATIONS_BRANCH];
+ const params = useParams<Record<string, string | undefined>>();
+ const routeBranch = params[PARAMS.TRANSLATIONS_BRANCH];
@@
- const selected = routeBranch ? (urlBranch ? urlBranch : null) : defaultBranch;
+ const selected = routeBranch ? (urlBranch ?? null) : defaultBranch;
@@
- selectedName: routeBranch,
+ selectedName: selected?.name,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const urlBranch = routeBranch | |
| ? data?.find((b) => b.name === routeBranch) | |
| : undefined; | |
| const defaultBranch = loadableBranches.isFetched | |
| ? loadedBranches?.find((b) => b.isDefault) || defaultBranchObject | |
| : null; | |
| const selected = routeBranch ? (urlBranch ? urlBranch : null) : defaultBranch; | |
| return { | |
| branches: data, | |
| selected: selected, | |
| default: defaultBranch, | |
| selectedName: routeBranch, | |
| loadable: loadableBranches, | |
| }; | |
| // At the top of the file | |
| import { useParams } from 'react-router-dom'; | |
| @@ | |
| - const match = useRouteMatch(); | |
| const params = useParams<Record<string, string | undefined>>(); | |
| const routeBranch = params[PARAMS.TRANSLATIONS_BRANCH]; | |
| const urlBranch = routeBranch | |
| ? data?.find((b) => b.name === routeBranch) | |
| : undefined; | |
| const defaultBranch = loadableBranches.isFetched | |
| ? loadedBranches?.find((b) => b.isDefault) || defaultBranchObject | |
| : null; | |
| const selected = routeBranch ? (urlBranch ?? null) : defaultBranch; | |
| return { | |
| branches: data, | |
| selected: selected, | |
| default: defaultBranch, | |
| selectedName: selected?.name, | |
| loadable: loadableBranches, | |
| }; |
🤖 Prompt for AI Agents
In
webapp/src/views/projects/translations/context/services/useBranchesService.tsx
around lines 55 to 71, replace useRouteMatch usage with useParams to read the
branch param from the URL, derive selectedName from selected?.name (so it
becomes undefined when the branch is invalid) instead of returning the raw route
param, and simplify selected using nullish coalescing (e.g. selected = urlBranch
?? defaultBranch) while updating imports to import useParams from react-router
(and removing useRouteMatch). Ensure the returned selectedName is the actual
branch name or undefined when invalid and adjust any types accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchCopyIntegrationTest.kt (1)
89-105: Race condition in branch existence check remains unaddressed.The direct assertion at line 104 can race with asynchronous deletion. Wrap the branch existence check in a wait loop as previously suggested.
🧹 Nitpick comments (1)
ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchCopyIntegrationTest.kt (1)
122-123: Variable names are backwards in assertIsCopyOf.The variable naming is confusing:
mainTranslationsis assigned fromthis.translations(the branch/copy key), whilebranchTranslationsis assigned fromother.translations(the main/original key). This naming is backwards and appears in multiple places (translations, screenshots, code references).Consider renaming for clarity:
- val mainTranslations = this.translations - val branchTranslations = other.translations + val copyTranslations = this.translations + val originalTranslations = other.translationsApply similar changes to screenshot and code reference variables at lines 167-168 and 176-177.
Also applies to: 167-168, 176-177
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ee/backend/app/src/main/kotlin/io/tolgee/ee/service/branching/BranchCopyServiceSql.kt(1 hunks)ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchCopyIntegrationTest.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- ee/backend/app/src/main/kotlin/io/tolgee/ee/service/branching/BranchCopyServiceSql.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build backend 🏗️
- GitHub Check: Build frontend 🏗️
🔇 Additional comments (4)
ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchCopyIntegrationTest.kt (4)
22-44: LGTM! Test infrastructure properly configured.The SpringBootTest configuration with TRACE logging, repository autowiring, and test data setup are all correctly implemented.
46-72: LGTM! Comprehensive validation of branch copy functionality.The test properly validates branch creation, key count, key copying, and branch readiness state.
74-87: LGTM! Performance validation is appropriate.The performance test with 2000 keys and a 3-second threshold provides good regression protection for the copy operation.
107-119: LGTM! Helper methods are clean and focused.The helper methods appropriately encapsulate HTTP interactions for branch creation and deletion.
| val mainKeyScreenshots = keyScreenshotReferenceRepository.getAllByKeyIdIn(listOf(this.id)) | ||
| val branchKeyScreenshots = keyScreenshotReferenceRepository.getAllByKeyIdIn(listOf(other.id)) | ||
| mainKeyScreenshots.first().let { screenshotReference -> | ||
| val branchScreenshotReference = branchKeyScreenshots.first() | ||
| screenshotReference.screenshot.assert.isEqualTo(branchScreenshotReference.screenshot) | ||
| screenshotReference.originalText.assert.isEqualTo(branchScreenshotReference.originalText) | ||
| screenshotReference.positions.assert.isEqualTo(branchScreenshotReference.positions) | ||
| } |
There was a problem hiding this comment.
Add size check before accessing first screenshot.
Line 169 calls .first() on mainKeyScreenshots without verifying the collection is non-empty, which could cause NoSuchElementException. This is inconsistent with the code references section (line 179) which includes a size check.
Apply this diff:
val mainKeyScreenshots = keyScreenshotReferenceRepository.getAllByKeyIdIn(listOf(this.id))
val branchKeyScreenshots = keyScreenshotReferenceRepository.getAllByKeyIdIn(listOf(other.id))
+ mainKeyScreenshots.assert.hasSizeGreaterThan(0)
+ mainKeyScreenshots.assert.hasSize(branchKeyScreenshots.size)
mainKeyScreenshots.first().let { screenshotReference ->📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val mainKeyScreenshots = keyScreenshotReferenceRepository.getAllByKeyIdIn(listOf(this.id)) | |
| val branchKeyScreenshots = keyScreenshotReferenceRepository.getAllByKeyIdIn(listOf(other.id)) | |
| mainKeyScreenshots.first().let { screenshotReference -> | |
| val branchScreenshotReference = branchKeyScreenshots.first() | |
| screenshotReference.screenshot.assert.isEqualTo(branchScreenshotReference.screenshot) | |
| screenshotReference.originalText.assert.isEqualTo(branchScreenshotReference.originalText) | |
| screenshotReference.positions.assert.isEqualTo(branchScreenshotReference.positions) | |
| } | |
| val mainKeyScreenshots = keyScreenshotReferenceRepository.getAllByKeyIdIn(listOf(this.id)) | |
| val branchKeyScreenshots = keyScreenshotReferenceRepository.getAllByKeyIdIn(listOf(other.id)) | |
| mainKeyScreenshots.assert.hasSizeGreaterThan(0) | |
| mainKeyScreenshots.assert.hasSize(branchKeyScreenshots.size) | |
| mainKeyScreenshots.first().let { screenshotReference -> | |
| val branchScreenshotReference = branchKeyScreenshots.first() | |
| screenshotReference.screenshot.assert.isEqualTo(branchScreenshotReference.screenshot) | |
| screenshotReference.originalText.assert.isEqualTo(branchScreenshotReference.originalText) | |
| screenshotReference.positions.assert.isEqualTo(branchScreenshotReference.positions) | |
| } |
🤖 Prompt for AI Agents
In
ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchCopyIntegrationTest.kt
around lines 167 to 174, the test calls .first() on mainKeyScreenshots and
branchKeyScreenshots without verifying they contain elements; add an explicit
size check or assertion before accessing .first() (matching the style used
around line 179) — for example assertThat(mainKeyScreenshots).hasSize(1) (or
assertTrue/isNotEmpty) and similarly for branchKeyScreenshots, then safely call
.first() (or use firstOrNull with assertNotNull) and keep the subsequent field
equality assertions.
… handle branch in services
…load (branch name in url)
…on-deleted branch
dkrizan
force-pushed
the
danielkrizan/branching
branch
from
0c36436 to
28f666d
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/data/src/main/kotlin/io/tolgee/dtos/IExportParams.kt (1)
109-125: Add missingfilterBranchassignment incopyPropsFrom
InIExportParams.kt, includethis.filterBranch = other.filterBranchinside
copyPropsFromto preserve branch filtering.
♻️ Duplicate comments (20)
backend/data/src/main/kotlin/io/tolgee/service/branching/BranchServiceOssStub.kt (1)
8-23: The critical bean collision issue has already been identified.The previous review comment comprehensively covers the missing
@ConditionalOnMissingBeanannotation that prevents bean conflicts in EE builds, as well as the improvement of exception messages. Please refer to the existing comment for the complete fix.webapp/src/eeSetup/eeModule.ee.tsx (1)
404-412: Protect the branches menu behind real permissionsLeaving
condition: ({ satisfiesPermission }) => trueships the branches menu (and its CRUD view) to every project member, bypassing authorization entirely. Wire the actual permission (and optional feature flag) before merging.backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2KeyController/KeyControllerKeySearchTest.kt (1)
150-155: Fix grammar of the test name.Please rename the display name to
it searches in default branch onlyto stay consistent with the naming pattern used across this suite and to resolve the earlier feedback.webapp/src/component/branching/BranchSelector.tsx (1)
17-23: Move side effect to useEffect to avoid render-phase mutations.The redirect logic executes during render, which can cause React warnings about state updates during render and potential infinite loops.
As noted in the previous review, wrap this in useEffect:
+ React.useEffect(() => { if (!loadable.isLoading && !selected) { history.replace( LINKS.PROJECT_TRANSLATIONS.build({ [PARAMS.PROJECT_ID]: project.id, }) ); } + }, [loadable.isLoading, selected, history, project.id]);backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/TestDataService.kt (1)
465-470: Fix save order to prevent FK violations.Persisting code references before their parent KeyMeta entities can break referential integrity constraints. The KeyMeta must be saved first to establish the FK target before saving code references.
As noted in the previous review, change the save order:
private fun saveAllKeyDependants(keyBuilders: List<KeyBuilder>) { val metas = keyBuilders.map { it.data.meta?.self }.filterNotNull() tagService.saveAll(metas.flatMap { it.tags }) - keyCodeReferenceRepository.saveAll(metas.flatMap { it.codeReferences }) keyMetaService.saveAll(metas) + keyMetaService.saveAllCodeReferences(metas.flatMap { it.codeReferences }) }This aligns with the import path at line 476 which uses
keyMetaService.saveAllCodeReferences. After this change,keyCodeReferenceRepositorydependency becomes unused and should be removed.backend/data/src/main/kotlin/io/tolgee/service/bigMeta/KeysDistanceUtil.kt (1)
46-52: Use item2.branch for key2 lookup (not item1.branch).This mixes branches when resolving
key2Id, producing wrong distances across branches.- val key2Id = - getKeyId(item2.namespace, item2.keyName, item1.branch ?: project.getDefaultBranch()?.name) ?: return@forEach2 + val key2Id = + getKeyId(item2.namespace, item2.keyName, item2.branch ?: project.getDefaultBranch()?.name) ?: return@forEach2backend/data/src/main/kotlin/io/tolgee/model/Project.kt (1)
148-150: This issue was previously flagged in a past review comment. The@SQLRestriction("archived_at IS NULL")references a column that must be mapped in theBranchentity. Based on test data (e.g.,BranchTestData.ktline 39, 127), thearchivedAtproperty appears to exist inBranch, but the past review requested verification that the column mapping is correct.ee/backend/app/src/main/kotlin/io/tolgee/ee/repository/BranchRepository.kt (1)
12-20: This issue was flagged in a past review. Thesearchparameter is unused in the query, andPageable?should likely be non-nullable. The past review suggested adding a search filter to the query and changing the signature to accept non-nullPageable.webapp/src/component/branching/BranchSelect.tsx (1)
45-50: This issue was flagged in a past review. TheuseEffectdepends only ondefaultBranch, but it should depend ondefaultValue(andonDefaultValue) to properly sync when thebranchprop changes. The past review suggested updating the dependency array.webapp/src/views/projects/translations/context/services/useBranchesService.tsx (4)
26-30: This issue was flagged in a past review and by static analysis. The conditional hook callprojectId || useProject().idviolates React hooks rules. The past review suggested callinguseProject()unconditionally and then deriving the resolved ID.
32-44: This issue was flagged in a past review. Theenabledprop is accepted but not passed to the query options, so the query can't be conditionally disabled. The past review suggested addingenabledto the options object.
46-54: This issue was flagged in a past review. The code uses a non-null assertion (loadedBranches!) which can throw when the query errors (sinceisFetchedcan be true even whendatais undefined). The past review suggested usingisSuccessand optional chaining.
55-71: This issue was flagged in a past review. TheselectedNamereturns the raw URL parameter even when the branch doesn't exist (selected becomes null). The past review suggested returningselected?.nameinstead and usinguseParamsrather thanuseRouteMatch.backend/data/src/main/kotlin/io/tolgee/repository/KeyRepository.kt (1)
288-305: Risk of MultipleBagFetchExceptionThis query fetch-joins multiple collections (
translations,t.labels,t.comments,km.tags) which can cause Hibernate's MultipleBagFetchException or cartesian product issues.ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchCopyIntegrationTest.kt (2)
102-109: Wrap branch existence check in wait loopDeletion is event-driven and may race. The branch existence check on line 108 should be wrapped in
waitForNotThrowingsimilar to the key count check to avoid test flakiness.
171-178: Add size check before accessing first screenshot.Calling
.first()onmainKeyScreenshotsandbranchKeyScreenshotswithout verifying the collections are non-empty can throwNoSuchElementException.backend/data/src/main/kotlin/io/tolgee/model/branching/Branch.kt (1)
41-56: Nullability mismatch for name and pending can cause NPEsKotlin fields
name: Stringandpending: Booleanare non-null, but@Columnannotations lacknullable = false. This mismatch can cause NPEs during entity hydration if DB columns allow NULL.backend/data/src/main/kotlin/io/tolgee/service/key/KeyService.kt (1)
242-247: Early return ignores branch contextThe early return only checks
nameandnamespaceequality, ignoring thebranchparameter. If branch changes while name/namespace remain the same, necessary work (uniqueness check, branch assignment) is skipped.backend/data/src/main/resources/db/changelog/schema.xml (1)
4658-4662: Still missing NOT NULL/default on branch.name & pendingBranch.name and Branch.pending are non-nullable in Kotlin (pending defaults to false), but the DDL still leaves both columns nullable and pending without a default. That leaks NULLs into an object graph the rest of the code treats as non-null, risking runtime NPEs and violating business rules. Please enforce the constraints and default directly in the changelog.
- <column name="name" type="VARCHAR(200)"/> + <column name="name" type="VARCHAR(200)"> + <constraints nullable="false"/> + </column> ... - <addColumn tableName="branch"> - <column name="pending" type="BOOLEAN"/> - </addColumn> + <addColumn tableName="branch"> + <column name="pending" type="BOOLEAN" defaultValueBoolean="false"> + <constraints nullable="false"/> + </column> + </addColumn>Because the table is new in this release, no backfill changeSet is required after setting the default.
Also applies to: 4701-4703
webapp/src/service/apiSchema.generated.ts (1)
2493-2497: Map new branch-related error codes in UIAdd UX mappings for: unsupported_media_type, branch_not_found, cannot_delete_default_branch, branch_already_exists, origin_branch_not_found.
#!/bin/bash # Look for error-code mapping usage rg -nP --type=ts -C3 'branch_not_found|cannot_delete_default_branch|branch_already_exists|origin_branch_not_found|unsupported_media_type'Also applies to: 5592-5596
🧹 Nitpick comments (4)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/dataImport/ImportTestData.kt (1)
354-354: Remove redundant author assignments.The
author = userAccountassignments inside theaddCodeReferencelambda blocks are redundant becauseuserAccountis already passed as the first parameter toaddCodeReference(userAccount), which sets the author via the constructor. Setting it again in the lambda body is unnecessary and violates DRY.Apply this diff to remove the redundant assignments:
addCodeReference(userAccount) { path = "./code/exist.extension" line = 10 fromImport = true - author = userAccount }addCodeReference(userAccount) { path = "./code/exist.extension" line = 10 fromImport = true - author = userAccount }addCodeReference(userAccount) { path = "./code/notExist.extendison" fromImport = false - author = userAccount }addCodeReference(userAccount) { path = "./code/exist.extension" line = 10 fromImport = true - author = userAccount }Also applies to: 375-375, 380-380, 393-393
webapp/src/component/branching/BranchLabel.tsx (1)
21-21: Avoidanytype assertion.The type assertion
(rest as any)bypasses TypeScript's type checking. Define a proper type for the component props or use a more specific type.Apply this diff:
-export const BranchLabel = React.forwardRef(function BranchLabel( - { value, placeholder, ...rest }: InputBaseComponentProps, - ref -) { +export const BranchLabel = React.forwardRef< + HTMLDivElement, + InputBaseComponentProps +>(function BranchLabel({ value, placeholder, ...rest }, ref) { return ( <StyledFakeInput tabIndex={0} ref={ref} style={{ display: 'flex' }} - {...(rest as any)} + {...rest} > <GitBranch02 height={20} width={20} style={{ marginRight: 10 }} /> <div>{value || placeholder}</div> </StyledFakeInput> ); });backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/TranslationsTestData.kt (2)
88-91: Disambiguate receiver: use[email protected]for clarity.Current
apply { branch = self }relies on implicit receiver resolution; make it explicit to avoid confusion in DSLs with multiple receivers.- addBasicKey().apply { - branch = self - } + addBasicKey().apply { + branch = [email protected] + }
135-136: Redundant manual list mutation; likely duplicates the translation in memory.
addTranslation { ... }should already manage the relationship. Manually addingself.translations.add(this)risks duplicates in the list.- self.translations.add(this)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (104)
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/keys/KeyController.kt(2 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/translation/CreateOrUpdateTranslationsFacade.kt(2 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/translation/TranslationsController.kt(2 hunks)backend/api/src/main/kotlin/io/tolgee/component/KeyComplexEditHelper.kt(1 hunks)backend/api/src/main/kotlin/io/tolgee/controllers/ExportController.kt(1 hunks)backend/api/src/main/kotlin/io/tolgee/hateoas/contentDelivery/ContentDeliveryConfigModel.kt(1 hunks)backend/api/src/main/kotlin/io/tolgee/hateoas/key/KeyModel.kt(1 hunks)backend/api/src/main/kotlin/io/tolgee/hateoas/key/KeyModelAssembler.kt(1 hunks)backend/api/src/main/kotlin/io/tolgee/hateoas/key/KeyWithDataModel.kt(1 hunks)backend/api/src/main/kotlin/io/tolgee/hateoas/key/KeyWithDataModelAssembler.kt(1 hunks)backend/api/src/main/kotlin/io/tolgee/hateoas/key/KeyWithScreenshotsModelAssembler.kt(1 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/AllKeysControllerTest.kt(1 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translations/v2TranslationsController/TranslationsControllerModificationTest.kt(2 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translations/v2TranslationsController/TranslationsControllerViewTest.kt(1 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ImportController/SingleStepImportControllerTest.kt(1 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2KeyController/KeyControllerCreationTest.kt(2 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2KeyController/KeyControllerKeySearchTest.kt(1 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2KeyController/KeyControllerTest.kt(4 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ProjectsController/ProjectsControllerCreateTest.kt(2 hunks)backend/app/src/test/kotlin/io/tolgee/service/LanguageServiceTest.kt(1 hunks)backend/app/src/test/kotlin/io/tolgee/service/TranslationServiceTest.kt(3 hunks)backend/data/src/main/kotlin/io/tolgee/constants/Message.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/TestDataService.kt(5 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/BranchBuilder.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/ProjectBuilder.kt(3 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/BranchTestData.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/BranchTranslationsTestData.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/KeySearchTestData.kt(3 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/KeysTestData.kt(3 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/TranslationsTestData.kt(4 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/dataImport/ImportTestData.kt(3 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/ExportParamsDocs.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/IExportParams.kt(2 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/RelatedKeyDto.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/queryResults/KeyIdFindResult.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/queryResults/KeyView.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/request/ContentDeliveryConfigRequest.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/request/export/ExportParams.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/request/key/ComplexEditKeyDto.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/request/key/CreateKeyDto.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/request/key/EditKeyDto.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/request/translation/SetTranslationsWithKeyDto.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/request/translation/TranslationFilters.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/request/translation/importKeysResolvable/ImportKeysResolvableItemDto.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/events/OnBranchDeleted.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/model/Project.kt(4 hunks)backend/data/src/main/kotlin/io/tolgee/model/branching/Branch.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/model/contentDelivery/ContentDeliveryConfig.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/model/key/Key.kt(4 hunks)backend/data/src/main/kotlin/io/tolgee/model/translation/Translation.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/repository/KeyMetaRepository.kt(2 hunks)backend/data/src/main/kotlin/io/tolgee/repository/KeyRepository.kt(9 hunks)backend/data/src/main/kotlin/io/tolgee/repository/TranslationRepository.kt(2 hunks)backend/data/src/main/kotlin/io/tolgee/service/bigMeta/BigMetaService.kt(3 hunks)backend/data/src/main/kotlin/io/tolgee/service/bigMeta/KeysDistanceUtil.kt(4 hunks)backend/data/src/main/kotlin/io/tolgee/service/branching/BranchCopyService.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/service/branching/BranchService.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/service/branching/BranchServiceOssStub.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/service/export/dataProvider/ExportDataProvider.kt(5 hunks)backend/data/src/main/kotlin/io/tolgee/service/key/KeyService.kt(6 hunks)backend/data/src/main/kotlin/io/tolgee/service/key/ResolvingKeyImporter.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/service/project/ProjectCreationService.kt(2 hunks)backend/data/src/main/kotlin/io/tolgee/service/queryBuilders/LanguageStatsProvider.kt(4 hunks)backend/data/src/main/kotlin/io/tolgee/service/queryBuilders/ProjectStatsProvider.kt(2 hunks)backend/data/src/main/kotlin/io/tolgee/service/queryBuilders/translationViewBuilder/QueryGlobalFiltering.kt(3 hunks)backend/data/src/main/kotlin/io/tolgee/service/translation/TranslationService.kt(2 hunks)backend/data/src/main/resources/db/changelog/schema.xml(1 hunks)backend/data/src/test/kotlin/io/tolgee/unit/bigMeta/KeysDistanceUnitTestData.kt(1 hunks)e2e/cypress/support/dataCyType.d.ts(2 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchController.kt(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/hateoas/assemblers/BranchModelAssembler.kt(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/hateoas/model/branching/BranchModel.kt(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/hateoas/model/branching/CreateBranchModel.kt(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/repository/BranchRepository.kt(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/service/branching/BranchCleanupService.kt(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/service/branching/BranchCopyServiceSql.kt(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/service/branching/BranchServiceImpl.kt(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/service/branching/DefaultBranchCreator.kt(1 hunks)ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchControllerTest.kt(1 hunks)ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchCopyIntegrationTest.kt(1 hunks)webapp/src/component/branching/BranchLabel.tsx(1 hunks)webapp/src/component/branching/BranchSelect.tsx(1 hunks)webapp/src/component/branching/BranchSelectField.tsx(1 hunks)webapp/src/component/branching/BranchSelector.tsx(1 hunks)webapp/src/component/branching/DefaultBranchChip.tsx(1 hunks)webapp/src/component/common/ClipboardCopy.tsx(1 hunks)webapp/src/component/searchSelect/InfiniteSearchSelect.tsx(6 hunks)webapp/src/constants/GlobalValidationSchema.tsx(1 hunks)webapp/src/constants/links.tsx(3 hunks)webapp/src/ee/branching/BranchesView.tsx(1 hunks)webapp/src/ee/branching/components/BranchForm.tsx(1 hunks)webapp/src/ee/branching/components/BranchItem.tsx(1 hunks)webapp/src/ee/branching/components/BranchModal.tsx(1 hunks)webapp/src/ee/branching/components/BranchNameChip.tsx(1 hunks)webapp/src/ee/branching/components/BranchesList.tsx(1 hunks)webapp/src/eeSetup/eeModule.ee.tsx(4 hunks)webapp/src/service/apiSchema.generated.ts(36 hunks)webapp/src/views/projects/ProjectRouter.tsx(1 hunks)webapp/src/views/projects/translations/KeyCreateForm/KeyCreateForm.tsx(3 hunks)webapp/src/views/projects/translations/TranslationHeader/TranslationControls.tsx(2 hunks)webapp/src/views/projects/translations/context/TranslationsContext.ts(3 hunks)webapp/src/views/projects/translations/context/services/useBranchesService.tsx(1 hunks)webapp/src/views/projects/translations/context/services/useEditService.tsx(3 hunks)webapp/src/views/projects/translations/context/services/useTranslationsService.tsx(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- webapp/src/ee/branching/components/BranchItem.tsx
🚧 Files skipped from review as they are similar to previous changes (50)
- backend/data/src/main/kotlin/io/tolgee/dtos/request/translation/SetTranslationsWithKeyDto.kt
- backend/data/src/main/kotlin/io/tolgee/dtos/ExportParamsDocs.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2ImportController/SingleStepImportControllerTest.kt
- backend/data/src/main/kotlin/io/tolgee/dtos/RelatedKeyDto.kt
- backend/data/src/main/kotlin/io/tolgee/service/queryBuilders/translationViewBuilder/QueryGlobalFiltering.kt
- backend/data/src/main/kotlin/io/tolgee/dtos/queryResults/KeyView.kt
- webapp/src/views/projects/translations/context/services/useEditService.tsx
- backend/api/src/main/kotlin/io/tolgee/hateoas/key/KeyModelAssembler.kt
- backend/api/src/main/kotlin/io/tolgee/hateoas/key/KeyWithDataModel.kt
- backend/data/src/main/kotlin/io/tolgee/dtos/request/export/ExportParams.kt
- webapp/src/views/projects/translations/KeyCreateForm/KeyCreateForm.tsx
- backend/app/src/test/kotlin/io/tolgee/service/LanguageServiceTest.kt
- webapp/src/views/projects/translations/context/TranslationsContext.ts
- backend/data/src/main/kotlin/io/tolgee/service/project/ProjectCreationService.kt
- webapp/src/views/projects/ProjectRouter.tsx
- backend/data/src/main/kotlin/io/tolgee/dtos/request/key/EditKeyDto.kt
- backend/api/src/main/kotlin/io/tolgee/hateoas/key/KeyWithScreenshotsModelAssembler.kt
- backend/data/src/main/kotlin/io/tolgee/dtos/request/translation/TranslationFilters.kt
- webapp/src/views/projects/translations/context/services/useTranslationsService.tsx
- e2e/cypress/support/dataCyType.d.ts
- webapp/src/ee/branching/components/BranchNameChip.tsx
- backend/data/src/main/kotlin/io/tolgee/service/branching/BranchService.kt
- backend/data/src/main/kotlin/io/tolgee/constants/Message.kt
- backend/data/src/main/kotlin/io/tolgee/dtos/request/translation/importKeysResolvable/ImportKeysResolvableItemDto.kt
- backend/data/src/main/kotlin/io/tolgee/repository/KeyMetaRepository.kt
- backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/KeySearchTestData.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/AllKeysControllerTest.kt
- backend/data/src/main/kotlin/io/tolgee/model/translation/Translation.kt
- ee/backend/app/src/main/kotlin/io/tolgee/ee/service/branching/BranchCopyServiceSql.kt
- webapp/src/constants/GlobalValidationSchema.tsx
- backend/data/src/main/kotlin/io/tolgee/dtos/request/key/CreateKeyDto.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2KeyController/KeyControllerCreationTest.kt
- backend/api/src/main/kotlin/io/tolgee/hateoas/key/KeyModel.kt
- ee/backend/app/src/main/kotlin/io/tolgee/ee/service/branching/BranchServiceImpl.kt
- ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/hateoas/assemblers/BranchModelAssembler.kt
- backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/BranchTranslationsTestData.kt
- webapp/src/views/projects/translations/TranslationHeader/TranslationControls.tsx
- backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/ProjectBuilder.kt
- backend/data/src/main/kotlin/io/tolgee/dtos/request/key/ComplexEditKeyDto.kt
- webapp/src/ee/branching/components/BranchesList.tsx
- backend/data/src/main/kotlin/io/tolgee/service/queryBuilders/LanguageStatsProvider.kt
- webapp/src/component/branching/DefaultBranchChip.tsx
- backend/data/src/main/kotlin/io/tolgee/service/key/ResolvingKeyImporter.kt
- ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchController.kt
- ee/backend/app/src/main/kotlin/io/tolgee/ee/service/branching/BranchCleanupService.kt
- webapp/src/ee/branching/components/BranchModal.tsx
- backend/api/src/main/kotlin/io/tolgee/hateoas/contentDelivery/ContentDeliveryConfigModel.kt
- backend/data/src/main/kotlin/io/tolgee/dtos/request/ContentDeliveryConfigRequest.kt
- webapp/src/component/common/ClipboardCopy.tsx
- backend/app/src/test/kotlin/io/tolgee/service/TranslationServiceTest.kt
🧰 Additional context used
🧬 Code graph analysis (16)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/dataImport/ImportTestData.kt (1)
backend/data/src/main/kotlin/io/tolgee/model/key/KeyMeta.kt (1)
addCodeReference(66-74)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/TranslationsTestData.kt (3)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/ProjectBuilder.kt (6)
addBranch(90-90)addKey(82-82)addKey(105-111)addKey(113-115)addKey(117-128)addTranslation(130-130)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/KeyBuilder.kt (1)
setDescription(100-103)backend/data/src/main/kotlin/io/tolgee/model/key/KeyMeta.kt (1)
addComment(56-64)
webapp/src/ee/branching/BranchesView.tsx (3)
webapp/src/views/projects/BaseProjectView.tsx (1)
BaseProjectView(19-60)webapp/src/constants/links.tsx (1)
LINKS(69-441)webapp/src/ee/branching/components/BranchesList.tsx (1)
BranchesList(21-135)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/KeysTestData.kt (2)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/ProjectBuilder.kt (1)
addBranch(90-90)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/KeyBuilder.kt (1)
setDescription(100-103)
webapp/src/component/branching/BranchSelect.tsx (5)
webapp/src/views/projects/translations/context/services/useBranchesService.tsx (1)
useBranchesService(26-72)webapp/src/component/searchSelect/SearchSelect.tsx (1)
SelectItem(8-11)webapp/src/component/branching/DefaultBranchChip.tsx (1)
DefaultBranchChip(5-8)webapp/src/component/searchSelect/InfiniteSearchSelect.tsx (1)
InfiniteSearchSelect(58-209)webapp/src/component/branching/BranchLabel.tsx (1)
BranchLabel(12-27)
backend/data/src/main/kotlin/io/tolgee/service/key/KeyService.kt (2)
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/translation/CreateOrUpdateTranslationsFacade.kt (1)
create(39-54)backend/data/src/main/kotlin/io/tolgee/model/key/Key.kt (1)
Key(41-131)
backend/data/src/main/kotlin/io/tolgee/service/export/dataProvider/ExportDataProvider.kt (1)
backend/data/src/main/kotlin/io/tolgee/service/queryBuilders/translationViewBuilder/QueryGlobalFiltering.kt (1)
filterBranch(267-284)
webapp/src/component/branching/BranchSelector.tsx (4)
webapp/src/service/apiSchema.generated.ts (1)
components(1072-6105)webapp/src/views/projects/translations/context/services/useBranchesService.tsx (1)
useBranchesService(26-72)webapp/src/constants/links.tsx (1)
LINKS(69-441)webapp/src/component/branching/BranchSelect.tsx (1)
BranchSelect(25-92)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/BranchTestData.kt (2)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/ProjectBuilder.kt (5)
addBranch(90-90)addKey(82-82)addKey(105-111)addKey(113-115)addKey(117-128)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/TestDataBuilder.kt (1)
addProject(53-61)
webapp/src/component/branching/BranchSelectField.tsx (1)
webapp/src/component/branching/BranchSelect.tsx (1)
BranchSelect(25-92)
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/translation/CreateOrUpdateTranslationsFacade.kt (1)
backend/data/src/main/kotlin/io/tolgee/service/key/KeyService.kt (2)
create(123-157)create(192-202)
webapp/src/ee/branching/components/BranchForm.tsx (3)
webapp/src/component/common/form/StandardForm.tsx (1)
StandardForm(37-121)webapp/src/constants/GlobalValidationSchema.tsx (1)
Validation(44-536)webapp/src/component/branching/BranchSelectField.tsx (1)
BranchSelectField(4-15)
webapp/src/eeSetup/eeModule.ee.tsx (2)
webapp/src/constants/links.tsx (1)
LINKS(69-441)webapp/src/ee/branching/BranchesView.tsx (1)
BranchesView(7-27)
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translations/v2TranslationsController/TranslationsControllerModificationTest.kt (1)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/TestDataService.kt (2)
saveTestData(77-83)saveTestData(85-121)
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translations/v2TranslationsController/TranslationsControllerViewTest.kt (1)
backend/testing/src/main/kotlin/io/tolgee/fixtures/ProjectApiKeyAuthRequestPerformer.kt (1)
performProjectAuthGet(47-49)
webapp/src/views/projects/translations/context/services/useBranchesService.tsx (2)
webapp/src/views/projects/translations/context/services/useTranslationsService.tsx (1)
useTranslationsService(74-435)webapp/src/service/http/useQueryApi.ts (1)
useApiInfiniteQuery(95-117)
🪛 Biome (2.1.2)
webapp/src/views/projects/translations/context/services/useBranchesService.tsx
[error] 30-30: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build frontend 🏗️
- GitHub Check: Build backend 🏗️
🔇 Additional comments (46)
backend/data/src/main/kotlin/io/tolgee/dtos/IExportParams.kt (2)
22-22: LGTM!The import for
FILTER_BRANCH_DESCRIPTIONis correctly added to support the new schema annotation.
104-107: LGTM!The new
filterBranchproperty is correctly declared with proper nullability and OpenAPI documentation.backend/data/src/main/kotlin/io/tolgee/model/contentDelivery/ContentDeliveryConfig.kt (1)
137-138: LGTM!The
filterBranchproperty follows the established pattern for filter properties in this class. It's properly annotated with@ActivityLoggedPropfor activity tracking and correctly overridesIExportParams.webapp/src/component/branching/BranchLabel.tsx (1)
18-18: ReconsidertabIndex={0}for a label component.Making a label component focusable with
tabIndex={0}may confuse users, as labels typically aren't interactive. If the component needs to be focusable for a specific reason (e.g., for accessibility with screen readers in a read-only context), consider addingrole="presentation"orrole="text"to clarify its semantic purpose.Verify whether this component is intended to be interactive. If it's purely presentational, remove the
tabIndex.ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/hateoas/model/branching/BranchModel.kt (1)
1-20: LGTM!The
BranchModelis well-structured with clear property descriptions. The HATEOAS annotations are properly applied for API documentation.backend/api/src/main/kotlin/io/tolgee/controllers/ExportController.kt (1)
64-70: LGTM!The addition of the
nullparameter togetTranslationsaligns with the branch-aware translation service signature introduced in this PR. Thenullvalue appropriately indicates no branch filtering for this export endpoint.backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/translation/CreateOrUpdateTranslationsFacade.kt (2)
35-35: LGTM!The addition of
dto.branchparameter tokeyService.findcorrectly propagates branch context for key resolution, aligning with the branch-aware architecture introduced in this PR.
77-77: LGTM!The addition of
dto.branchparameter tokeyService.getmaintains consistency with the branch-aware key lookup pattern.backend/data/src/main/kotlin/io/tolgee/events/OnBranchDeleted.kt (1)
1-7: LGTM!The
OnBranchDeletedevent class is simple and correctly structured for event-driven branch cleanup integration.backend/data/src/main/kotlin/io/tolgee/repository/TranslationRepository.kt (1)
25-45: LGTM!The branch filtering logic is correctly implemented:
- When branch parameter is provided: matches only that specific branch (and ensures it's not archived)
- When branch parameter is null: matches keys with no branch association or keys on the default branch
This aligns with the branch-aware filtering pattern used in other queries across the codebase.
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/TestDataService.kt (1)
598-602: LGTM!The
saveBranchesmethod correctly persists unpersisted Branch entities (whereid == 0L) and integrates properly into the project data saving flow.webapp/src/constants/links.tsx (1)
66-66: LGTM!The new routing constants correctly support branch-aware navigation:
TRANSLATIONS_BRANCHparam for branch name in URLsPROJECT_TRANSLATIONS_BRANCHEDfor branched translation viewsPROJECT_BRANCHESfor branch management UIThese align with the broader branching feature implementation.
Also applies to: 352-355, 364-365
backend/data/src/main/kotlin/io/tolgee/service/translation/TranslationService.kt (1)
72-93: LGTM!The branch parameter is correctly threaded through to the repository layer, enabling branch-aware translation retrieval. The implementation properly handles the namespace normalization and passes the branch context to
translationRepository.getTranslations.This aligns with the branch-aware filtering introduced across the data access layer.
backend/data/src/main/kotlin/io/tolgee/service/export/dataProvider/ExportDataProvider.kt (1)
53-53: LGTM!The branch filtering implementation is correct and consistent with the pattern used in other queries:
- When
filterBranchis specified: matches only that specific branch (and ensures it's not archived)- When
filterBranchis null: matches keys with no branch or keys on the default branchThe join initialization and filter invocation are properly integrated into the query construction flow.
Also applies to: 70-70, 97-97, 158-174
webapp/src/component/branching/BranchSelectField.tsx (1)
1-15: LGTM!The Formik integration is correctly implemented:
- Properly uses
useFieldanduseFormikContexthooks- Handles field type conversion (
null→undefined) correctly- Both
onDefaultValueandonSelectcallbacks appropriately set the field value tobranch.idThe component provides clean integration between BranchSelect and Formik form state management.
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/BranchBuilder.kt (1)
5-10: LGTM: simple builder correctly links Branch to Project.backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/v2KeyController/KeyControllerTest.kt (1)
47-48: Tests correctly validate default vs. branch-scoped key listing.
- Adding branched keys should not affect default listing count (still 123).
- New
returns all keys from branchcovers branch filtering and pagination.Also applies to: 58-59, 71-97
backend/data/src/main/kotlin/io/tolgee/model/key/Key.kt (1)
50-51: Enforce DB unique constraint on (namespace_id, name, branch_id)
No existing JPA or migration constraints referencebranch_id. Add or verify a unique constraint coveringnamespace_id,name, andbranch_idin both the@Table(uniqueConstraints=…)of Key.kt and the corresponding DB migration.backend/data/src/main/kotlin/io/tolgee/model/Project.kt (1)
188-194: LGTM: Helper methods correctly handle default branch logic.The
hasDefaultBranch()andgetDefaultBranch()methods provide clean abstractions for checking and retrieving the default branch from the filtered collection.backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/BranchTestData.kt (1)
1-53: LGTM: Well-structured test data builder for branching scenarios.The test data builder correctly sets up:
- A main default protected branch
- A feature branch with origin relationship
- An archived branch to test filtering behavior
- Clean separation with a second project for isolation testing
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/keys/KeyController.kt (2)
128-137: LGTM: Branch parameter correctly added to key listing.The optional
branchparameter enables branch-scoped key retrieval and is properly propagated to the service layer.
155-157: LGTM: Branch name correctly included in key view response.The branch name is safely extracted and included in the
KeyView, enabling clients to display branch context for edited keys.backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/KeysTestData.kt (2)
110-133: LGTM: Branch test data covers edge cases including archived branches.The test setup includes:
- A default main branch
- An active dev branch with keys
- An archived dev branch (same name) to verify filtering logic
The hardcoded timestamp for
archivedAt(Jan 1970) is appropriate for testing archived state.
157-173: LGTM: Helper function correctly creates branched keys for testing.The
addNBranchedKeysfunction provides a convenient way to generate test keys within a specific branch, properly linking the branch relationship and setting descriptions.ee/backend/app/src/main/kotlin/io/tolgee/ee/repository/BranchRepository.kt (2)
22-29: LGTM: Query correctly filters by project, branch ID, and archived state.The method appropriately returns a nullable
Branch?and includes the archived filter for consistency.
31-38: LGTM: Case-insensitive branch name lookup is correctly implemented.The query appropriately uses
lower()for case-insensitive matching and filters archived branches.webapp/src/component/branching/BranchSelect.tsx (4)
25-43: LGTM: Branch selection initialization handles both ID and object props.The component correctly resolves the default branch from either an explicit
branchprop (by ID or object match) or falls back to the service'sdefaultBranch.
52-66: LGTM: Item rendering correctly displays branch name and default indicator.The render function appropriately shows a chip for default branches and handles selection state.
68-71: LGTM: Selection handler correctly updates state and notifies parent.
73-91: LGTM: InfiniteSearchSelect configuration is correct for branch selection.The component properly wires up the infinite select with custom branch-specific rendering and labeling.
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translations/v2TranslationsController/TranslationsControllerModificationTest.kt (3)
394-413: LGTM: Test correctly verifies branch-specific translation updates.The test properly exercises setting translations for a key within a specific branch and validates the response structure.
415-427: LGTM: Test verifies branch context is required for branched keys.The test correctly ensures that attempting to modify a branched key without providing the branch parameter results in a 404.
429-442: LGTM: Test verifies branch isolation for translation updates.The test correctly ensures that attempting to modify a default-branch key while specifying a different branch results in a 404, maintaining branch isolation.
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translations/v2TranslationsController/TranslationsControllerViewTest.kt (3)
108-118: LGTM!Test correctly validates that only non-branched/default keys are returned when no branch parameter is specified.
120-131: LGTM!Test correctly validates that both non-branched keys and default branch keys are returned, totaling 7 keys as expected.
133-150: LGTM!Test correctly validates branch-specific filtering and includes thorough translation content verification.
ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchControllerTest.kt (1)
23-122: LGTM!Test suite comprehensively covers branch CRUD operations including edge cases (duplicate names, default branch protection, auto-creation of default branch).
webapp/src/component/searchSelect/InfiniteSearchSelect.tsx (1)
50-76: LGTM!Component enhancements improve flexibility by allowing custom input components, configurable menu positioning, and minimum height control. All props are correctly typed and integrated.
Also applies to: 120-120, 131-131, 135-139, 166-171
backend/data/src/main/kotlin/io/tolgee/repository/KeyRepository.kt (3)
20-32: LGTM!Branch-aware counting and ID pagination methods are correctly implemented.
34-51: LGTM!Branch filtering logic is correct: matches specified non-archived branch or defaults to null/default branch when branch is not specified.
60-71: LGTM!KeyView construction and branch filtering are consistently applied across all retrieval methods.
Also applies to: 154-175, 177-191, 267-277
ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchCopyIntegrationTest.kt (2)
50-76: LGTM!Test thoroughly validates branch copy functionality including key count and key comparison using the helper method.
78-91: LGTM!Performance test ensures branch copy operations scale efficiently with large datasets.
backend/data/src/main/kotlin/io/tolgee/service/key/KeyService.kt (1)
67-93: LGTM!Branch parameter is correctly threaded through key lookup, creation, validation, and pagination methods.
Also applies to: 193-222, 291-300, 406-410
webapp/src/service/apiSchema.generated.ts (2)
1349-1363: Branch model semantics — respect isDefault/isProtected and origin selectionUI should:
- Honor isDefault/isProtected when rendering actions (no delete/rename where prohibited).
- Require originBranchId on create (and validate existence).
- Show active status appropriately.
Also applies to: 1932-1940, 3758-3762
13415-13416: No modifications needed — all affected endpoints already includebranch?: stringin their request definitions.
| @Parameter(description = "Branch name to return translations from") | ||
| branch: String? = null, | ||
| request: WebRequest, |
There was a problem hiding this comment.
Annotate branch as a query param to ensure binding and OpenAPI docs.
Without @RequestParam, Spring may not bind it and OpenAPI may not render it as query param.
Apply:
@Parameter(description = "Branch name to return translations from")
- branch: String? = null,
+ @RequestParam(value = "branch", required = false)
+ branch: String? = null,🤖 Prompt for AI Agents
In
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/translation/TranslationsController.kt
around lines 163-165, the branch parameter isn't annotated so Spring won't bind
it as a query parameter and OpenAPI won't display it; annotate the parameter
with @RequestParam(required = false) (and optionally add @Parameter(description
= "Branch name to return translations from")) so it is treated as an optional
query parameter and appears in the generated OpenAPI docs.
| fun generateBranchedData(count: Long, branchName: String = "feature-branch", isBranchDefault: Boolean = false) { | ||
| root.data.projects[0].apply { | ||
| addBranch { | ||
| name = branchName | ||
| project = this@apply.self | ||
| isDefault = isBranchDefault | ||
| }.build { | ||
| (1..count).forEach { | ||
| addKey { | ||
| name = "key from branch $branchName $it" | ||
| branch = this@build.self | ||
| }.build { | ||
| addTranslation { | ||
| language = germanLanguage | ||
| text = "I am key $it's german translation from branch $branchName." | ||
| } | ||
| addTranslation { | ||
| language = englishLanguage | ||
| text = "I am key $it's english translation from branch $branchName." | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Kotlin range type bug: (1..count) with count: Long won’t compile.
Use a LongRange or change the parameter type. The same pattern elsewhere should be aligned.
Option A (keep Long):
- (1..count).forEach {
+ (1L..count).forEach {Option B (prefer Int counts in tests):
- fun generateBranchedData(count: Long, branchName: String = "feature-branch", isBranchDefault: Boolean = false) {
+ fun generateBranchedData(count: Int, branchName: String = "feature-branch", isBranchDefault: Boolean = false) {
...
- (1..count).forEach {
+ (1..count).forEach {🤖 Prompt for AI Agents
In
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/TranslationsTestData.kt
around lines 294-318, the for-range uses (1..count) while count is declared as
Long which doesn't compile; change the parameter type from Long to Int
(preferred for test counts) and update any call sites and other similar methods
in this file to use Int as well so the Kotlin IntRange (1..count) compiles, or
alternatively replace the range with (1L..count).forEach if you must keep Longs
— be consistent across the file.
| cb.or( | ||
| cb.and( | ||
| cb.equalNullable(branch.get(Branch_.name), key.branch), | ||
| cb.isNull(branch.get(Branch_.archivedAt)), | ||
| ), | ||
| cb.or( | ||
| cb.isNull(root.get(Key_.branch)), | ||
| cb.isTrue(branch.get(Branch_.isDefault)), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Fix branch filtering logic to use conditional if/else pattern.
The current OR condition incorrectly includes default branch keys when querying for a specific feature branch. When key.branch is "feature-1", the second condition (cb.isNull(root.get(Key_.branch)) OR cb.isTrue(branch.get(Branch_.isDefault))) would still match default branch keys, mixing results from different branches.
Compare with the correct pattern in ExportDataProvider.kt (lines 158-174) and QueryGlobalFiltering.kt (lines 266-283) which use if/else logic:
- If a branch is specified: match only that specific branch (and not archived)
- Else: match keys with no branch or default branch
Apply this diff to fix the logic:
- cb.or(
- cb.and(
- cb.equalNullable(branch.get(Branch_.name), key.branch),
- cb.isNull(branch.get(Branch_.archivedAt)),
- ),
- cb.or(
- cb.isNull(root.get(Key_.branch)),
- cb.isTrue(branch.get(Branch_.isDefault)),
- ),
- )
+ if (key.branch != null) {
+ cb.and(
+ cb.equal(branch.get(Branch_.name), key.branch),
+ cb.isNull(branch.get(Branch_.archivedAt)),
+ )
+ } else {
+ cb.or(
+ cb.isNull(root.get(Key_.branch)),
+ cb.isTrue(branch.get(Branch_.isDefault)),
+ )
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cb.or( | |
| cb.and( | |
| cb.equalNullable(branch.get(Branch_.name), key.branch), | |
| cb.isNull(branch.get(Branch_.archivedAt)), | |
| ), | |
| cb.or( | |
| cb.isNull(root.get(Key_.branch)), | |
| cb.isTrue(branch.get(Branch_.isDefault)), | |
| ), | |
| ) | |
| if (key.branch != null) { | |
| cb.and( | |
| cb.equal(branch.get(Branch_.name), key.branch), | |
| cb.isNull(branch.get(Branch_.archivedAt)), | |
| ) | |
| } else { | |
| cb.or( | |
| cb.isNull(root.get(Key_.branch)), | |
| cb.isTrue(branch.get(Branch_.isDefault)), | |
| ) | |
| } |
🤖 Prompt for AI Agents
In backend/data/src/main/kotlin/io/tolgee/service/bigMeta/BigMetaService.kt
around lines 238 to 247, the branch filter uses a single cb.or(...) that wrongly
allows default-branch keys to match even when a specific branch is requested;
change it to a conditional if/else: when key.branch is non-null, add only the
AND condition that matches the specific branch name and non-archived branch;
otherwise add the OR condition that matches root Key_.branch IS NULL or
Branch_.isDefault TRUE. Ensure the query builder uses the appropriate branch
predicate depending on the presence of key.branch so specific-branch queries do
not include default-branch keys.
| @Transactional | ||
| fun create(project: Project): Branch { | ||
| val branch = Branch.createMainBranch(project) | ||
| keyService.getAll(project.id).forEach { it -> | ||
| it.branch = branch | ||
| } | ||
| return branch | ||
| } |
There was a problem hiding this comment.
Bug: assigning transient Branch to persisted Keys (unsaved entity). Persist Branch first.
Branch.createMainBranch(project) creates a new entity but it's never saved. Assigning it to existing keys will cause TransientPropertyValueException on flush.
package io.tolgee.ee.service.branching
@@
-import io.tolgee.service.key.KeyService
+import io.tolgee.service.key.KeyService
import io.tolgee.service.project.ProjectService
+import io.tolgee.service.branching.BranchService
@@
class DefaultBranchCreator(
private val keyService: KeyService,
private val projectService: ProjectService,
+ private val branchService: BranchService,
) {
@@
@Transactional
fun create(project: Project): Branch {
- val branch = Branch.createMainBranch(project)
- keyService.getAll(project.id).forEach { it ->
- it.branch = branch
- }
- return branch
+ val created = Branch.createMainBranch(project)
+ val branch = branchService.save(created)
+ // TODO: Consider batch update for performance
+ keyService.getAll(project.id).forEach { key ->
+ key.branch = branch
+ }
+ return branch
}
}Also consider a bulk update method in KeyService/Repository to avoid N updates on large projects.
I can provide a JPQL/SQL batch update snippet if desired.
🤖 Prompt for AI Agents
In
ee/backend/app/src/main/kotlin/io/tolgee/ee/service/branching/DefaultBranchCreator.kt
around lines 22 to 29, the code creates a new Branch with
Branch.createMainBranch(project) but never persists it before assigning it to
existing Key entities, causing a TransientPropertyValueException on flush; fix
by saving the new branch via the Branch repository (or entityManager.persist)
immediately after creation so it has an id and managed state, then assign it to
keys; to avoid N updates for large projects, replace the per-entity loop with a
KeyService/Repository bulk update (JPQL/SQL update setting branch_id where
project_id = :projectId) or add a dedicated batch update method that runs a
single update query and flushes/clears appropriately.
| "/v2/projects/{projectId}/branches": { | ||
| get: operations["all"]; | ||
| post: operations["create_11"]; | ||
| }; | ||
| "/v2/projects/{projectId}/branches/{branchId}": { | ||
| delete: operations["delete_15"]; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
New branch endpoints — wire client + UI flows
Add service wrappers and UI for:
- GET/POST /v2/projects/{projectId}/branches (list/create)
- DELETE /v2/projects/{projectId}/branches/{branchId}
Ensure EE gating/permissions, and disable delete for default/protected branches (see schema flags).
Also applies to: 11558-11685
🏁 Script executed:
#!/bin/bash
# Verify client and UI use the new endpoints
rg -nP --type=ts -C2 '/v2/projects/\{?[^}]*\}?/branches\b' || true
rg -nP --type=ts -C2 '\bbranches?\b' webapp | sed -n '1,200p' || trueLength of output: 11688
Enforce EE permissions and disable deletion for protected branches
- Replace
canEditBranches = true(BranchesList.tsx:72) withsatisfiesPermission('branches.edit'). - In BranchesList.tsx, disable the delete action for default/protected branches using the schema’s
protected/isDefaultflag.
🤖 Prompt for AI Agents
In webapp/src/service/apiSchema.generated.ts around lines 413-419 and
BranchesList.tsx around line 72, replace the hardcoded canEditBranches = true
boolean with a runtime permission check using
satisfiesPermission('branches.edit') and update the UI action logic so the
delete action is disabled when the branch schema marks it as protected or
isDefault; specifically, use the branch object's protected or isDefault flag
from the generated schema to conditionally disable or hide the delete button and
ensure permission check gates edit/delete actions.
| /** @description Branch of the key. If not provided, default branch will be used */ | ||
| branch?: string; | ||
| /** @description Custom values of the key. If not provided, custom values won't be modified */ |
There was a problem hiding this comment.
🧩 Analysis chain
Pass branch in key create/edit/import/translation mutations
New optional branch fields were added across DTOs/models. Ensure mutation calls set branch when working on non‑default branches to avoid writing to default:
- ComplexEditKeyDto.branch
- CreateKeyDto.branch, EditKeyDto.branch
- ImportKeysResolvableItemDto.branch, RelatedKeyDto.branch
- KeyModel/KeyWithDataModel.branch (surface in UI)
- SetTranslationsWithKeyDto.branch
Also applies to: 1972-1972, 2111-2113, 2882-2882, 4814-4814, 3086-3091, 3175-3177, 5013-5015
🏁 Script executed:
#!/bin/bash
# Check mutation call sites set `branch` where appropriate
rg -nP --type=ts -C3 '\b(complexEdit|createOrUpdateTranslations|setTranslations)\b'
rg -nP --type=ts -C3 '\bbranch\s*:' Length of output: 9611
🏁 Script executed:
#!/bin/bash
# Show create key form submission code
sed -n '1,200p' webapp/src/views/projects/translations/KeyCreateForm/KeyCreateForm.tsxLength of output: 3961
🏁 Script executed:
#!/bin/bash
rg -nP --type=ts -C3 "url: '/v2/projects/{projectId}/keys/{id}/complex-update'" -n webapp/srcLength of output: 655
🏁 Script executed:
#!/bin/bash
sed -n '60,140p' webapp/src/views/projects/translations/KeyEdit/KeyEditModal.tsxLength of output: 2413
🏁 Script executed:
#!/bin/bash
# Show payload of updateKeyLoadable.mutateAsync to verify branch usage
rg -nP --type=ts "updateKeyLoadable\.mutateAsync" -A15 -n webapp/src/views/projects/translations/KeyEdit/KeyEditModal.tsxLength of output: 657
Add branch to complex-update mutation payload
In KeyEditModal.tsx (around lines 138–152), the updateKeyLoadable.mutateAsync call’s JSON body is missing branch and will always write to the default branch. Include the active branch (e.g. branchesService.selected?.name) under 'application/json' so edits target the correct non-default branch.
🤖 Prompt for AI Agents
In webapp/src/service/apiSchema.generated.ts around lines 1514 to 1516, the
complex-update mutation payload currently omits the branch property so edits
always target the default branch; add a branch field to the JSON body (under
'application/json') and populate it with the active branch (e.g.
branchesService.selected?.name) so the mutation sends { ..., branch:
branchesService.selected?.name, ... } and the update applies to the selected
non-default branch.
| /** | ||
| * @description Filter translations with branch. | ||
| * | ||
| * By default, default branch is exported. | ||
| */ | ||
| filterBranch?: string; | ||
| /** @description Filter key IDs to be contained in export */ |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Content delivery configs: branch filter added — plumb through
Expose filterBranch in create/update forms and when publishing so CDN can serve a specific branch.
Also applies to: 1796-1803
🏁 Script executed:
#!/bin/bash
# Check if content-delivery config UI passes filterBranch
rg -nP --type=ts -C3 'content-delivery-configs|filterBranch'Length of output: 8509
Expose filterBranch in content delivery config UI
Add a filterBranch field to the create/update form in CdDialog.tsx and include it in the request body for createCd, updateCd (useCdActions.tsx), and publish (CdItem.tsx).
| fileStructureTemplate?: string; | ||
| /** | ||
| * @description Filter translations with branch. | ||
| * | ||
| * By default, default branch is exported. | ||
| */ | ||
| filterBranch?: string; | ||
| /** @description Filter key IDs to be contained in export */ |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Export: support branch filtering
Add branch selection to export flows (GET /export and POST /export with ExportParams.filterBranch). Default remains default branch.
Also applies to: 12391-12397
🏁 Script executed:
#!/bin/bash
# Verify export calls and presence of branch support
rg -nP --type=ts -C3 '\bexportData\b|\bexportPost\b|/v2/projects/.*/export'
rg -nP --type=ts -C3 '\bfilterBranch\b'Length of output: 6118
Export: support branch selection in frontend export flows
The API schema now includes filterBranch for both GET (exportData) and POST (exportPost) /v2/projects/{projectId}/export. In OperationExportTranslations.tsx and ExportForm.tsx, pass the selected filterBranch (defaulting to the default branch) in the request payload. Update the e2e tests to assert that filterBranch is sent.
🤖 Prompt for AI Agents
In webapp/src/service/apiSchema.generated.ts around lines 2556 to 2563, the
generated API type now includes filterBranch for export endpoints but the
frontend export flows weren't wiring it through; update
OperationExportTranslations.tsx and ExportForm.tsx to include the selected
filterBranch in the request payload (falling back to the project's default
branch when no branch is selected), ensuring both GET exportData and POST
exportPost calls pass that property; then update the relevant e2e tests to
assert the request body/query includes filterBranch (and that default branch is
used when none is selected).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchCopyIntegrationTest.kt (2)
102-109: Wrap branch deletion assertion in wait loopBranch removal is event-driven; asserting immediately after the key-count wait can still race and flake. Please wrap the branch null check in the same
waitForNotThrowingpattern.- branchRepository.findByIdOrNull(testData.toBeDeletedBranch.id).assert.isNull() + waitForNotThrowing(timeout = 10000, pollTime = 250) { + branchRepository.findByIdOrNull(testData.toBeDeletedBranch.id).assert.isNull() + }
171-178: Ensure screenshot lists are non-empty before.first()
mainKeyScreenshots.first()andbranchKeyScreenshots.first()still riskNoSuchElementExceptionif either list is empty. Please add size/non-empty checks before dereferencing.- mainKeyScreenshots.first().let { screenshotReference -> + mainKeyScreenshots.assert.hasSize(branchKeyScreenshots.size) + mainKeyScreenshots.assert.isNotEmpty() + branchKeyScreenshots.assert.isNotEmpty() + mainKeyScreenshots.first().let { screenshotReference ->backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/TranslationsTestData.kt (1)
290-304: Fix Long range iteration
(1..count)still fails to compile whencountisLong; Kotlin only definesIntRangefor anIntliteral lower bound. Please switch to aLongRange(or changecounttoInt) so this helper builds.- (1..count).forEach { + (1L..count).forEach {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translations/v2TranslationsController/TranslationsControllerViewTest.kt(2 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/TranslationsTestData.kt(4 hunks)ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchCopyIntegrationTest.kt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/translations/v2TranslationsController/TranslationsControllerViewTest.kt (1)
backend/testing/src/main/kotlin/io/tolgee/fixtures/ProjectApiKeyAuthRequestPerformer.kt (1)
performProjectAuthGet(47-49)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/TranslationsTestData.kt (3)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/ProjectBuilder.kt (6)
addBranch(90-90)addKey(82-82)addKey(105-111)addKey(113-115)addKey(117-128)addTranslation(130-130)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/KeyBuilder.kt (1)
setDescription(100-103)backend/data/src/main/kotlin/io/tolgee/model/key/KeyMeta.kt (1)
addComment(56-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build backend 🏗️
| mainTranslation.comments.assert.hasSize(branchTranslation.comments.size) | ||
| mainTranslation.comments.first().let { comment -> | ||
| val branchComment = branchTranslation.comments.first() | ||
| comment.id.assert.isNotEqualTo(branchComment.id) | ||
| comment.text.assert.isEqualTo(branchComment.text) | ||
| comment.state.assert.isEqualTo(branchComment.state) | ||
| comment.author.assert.isEqualTo(branchComment.author) | ||
| } |
There was a problem hiding this comment.
Guard comment collections before calling .first()
mainTranslation.comments.first() (and the branch counterpart) will throw if the collection is empty. Add explicit size/non-empty assertions before accessing first() so the failure mode stays diagnostic instead of crashing the test.
- mainTranslation.comments.assert.hasSize(branchTranslation.comments.size)
- mainTranslation.comments.first().let { comment ->
+ mainTranslation.comments.assert.hasSize(branchTranslation.comments.size)
+ mainTranslation.comments.assert.isNotEmpty()
+ branchTranslation.comments.assert.isNotEmpty()
+ mainTranslation.comments.first().let { comment ->🤖 Prompt for AI Agents
In
ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/BranchCopyIntegrationTest.kt
around lines 161 to 168, the test calls mainTranslation.comments.first() and
branchTranslation.comments.first() without guarding against empty collections;
add explicit assertions that the collections are non-empty (or assert their
sizes > 0) before calling first(), or substitute first() usage with safe access
after asserting hasSize/nonEmpty so the test fails with a clear diagnostic
instead of throwing an exception.
Summary by CodeRabbit
New Features
Improvements
Tests
Chores